Improve LiveDebugValues for arm64 Swift async variables. #2769
Conversation
This patch contains two bugfixes: 1. iterate over the MachineRefInfo's livein values instead of the MBB 2. move all DBG_VALUEs that point into an asyn context to the very beginning of the first basic block rdar://75904857
|
@swift-ci test |
| for (auto R : EntryMBB.liveins()) { | ||
| if (R.PhysReg == Reg && F.hasParamAttribute(I, Attribute::SwiftAsync)) | ||
| for (auto R : MF->getRegInfo().liveins()) { | ||
| if (R.first == Reg && F.hasParamAttribute(I, Attribute::SwiftAsync)) |
There was a problem hiding this comment.
Might be clearer to do the F.hasParamAttribute(I, Attribute::SwiftAsync) check above, where we check F.getCallingConv().
Edit: Nvm, you need to iterate over the liveins to check this.
There was a problem hiding this comment.
Just curious - the only liveins field I see in the MIR is in the entry MBB; where do we pick up the extra liveins?
There was a problem hiding this comment.
Here is the concrete (and surprising!) example MIR that prompted this:
Function Live Ins: $x22, $x0
bb.0 (%ir-block.2):
successors: %bb.2, %bb.1
liveins: $x0, $x22, $lr
There was a problem hiding this comment.
Weird, so we actually want the extra livein for $lr?
Is it legitimate to exclude $lr from the MF's liveins list?
Also, what's up with the ordering of the liveins? Is it random? If $lr were the first entry in the liveins list, couldn't the F.hasParamAttribute(I, Attribute::SwiftAsync) check spuriously fail?
There was a problem hiding this comment.
@aemerson @ornata @TNorthover Do you what the best way would be to query which incoming register corresponds to which llvm::Argument? The condition I want to write here is: "is this the incoming register that the ABI reserves for the swift async context?"
| recordEntryValue(MI, DefinedRegs, OpenRanges, VarLocIDs); | ||
| } | ||
| } | ||
|
|
||
| if (AsyncDbgValues.size()) { | ||
| // Make sure the async entry values are at the very start. |
There was a problem hiding this comment.
Could you leave a comment explaining why this is needed?
If the DBG_VALUEs are already in the entry MBB, but LDV drops them accidentally if they're not present at the very start, that could be a separate LDV bug to fix.
There was a problem hiding this comment.
The problem that the register allocator inserts a few copy instructions before the DBG_VALUEs, and normally that would be the correct thing to do, but for virtual backtraces the DBG_VALUEs must be on the DW_AT_low_pc of the function, or the variables aren't visible. And since they directly refer to the incoming register, that's also correct.
There was a problem hiding this comment.
And apparently that only happens on arm64.
There was a problem hiding this comment.
I see, so there's some separate, async-specific requirement that the locations for the async arguments start at the low_pc of the function/funclet?
This patch contains two bugfixes:
rdar://75904857