I’m having an issue in a downstream target that I’m trying to track down and characterize involving a custom scheduling pass similar to Hexagon’s VLIW scheduler, and am looking for any tips on what I should be looking for.
Consider the following instructions taking place in program order in a MachineBasicBlock
Hi James,
I can’t imagine that this is expected. The knowledge that the use of $0 in the new 1) is ‘variable_name’ is lost. Furthermore, we now have a strange liveness problem in that the DBG_VALUE is apparently killing $r0, which sounds logically incorrect.
Unfortunately this is expected, even though it's highly undesirable.
Because DBG_VALUE instructions specify both the program position where
a variable is assigned a location, and the register location too,
correctly placing DBG_VALUEs after scheduling becomes extremely
difficult. If we prioritise placing them near the position they were
before scheduling we would often get the register location wrong. If
we focused on the register location always being correct, we would
then often get the program position wrong, re-ordering assignments. I
believe the current solution gets position/register-location wrong
occasionally, but not especially often.
Shameless plug: my instruction-referencing rewrite of variable
locations [0] would make this easier, as the concern about register
locations is separated, and we would only need to think about where in
the program to place DBG_INSTR_REF instructions after scheduling. It's
back in the landing-patches [1] phase and might be production ready
for llvm 14, but possibly not for VLIW machines, it's been a long time
since I've gone near them.
BUNDLE implicit $r0
use $r0
BUNDLE implicit $r0
DBG_VALUE $r0, $noreg, !”variable_name”After the fixup step, both bundles are marked as kills of $r0, I believe as a consequence of the fact that the fixup step skips debug instructions at the top level, but not at the bundle level. This triggers the MachineVerifier to complain that the bundle containing the DBG_VALUE is using an undefined register because the prior bundle killed it.
This sounds fixable -- all register uses in DBG_VALUE / DBG_VALUE_LIST
instructions should have the "IsDebug" flag set on them, which should
allow the verifier to distinguish between "the code is broken" and
"the debug-info is faulty", the latter being non-fatal. I've no
familiarity with instruction bundles, but I'd guess the "IsDebug" flag
can be propagated to their uses, or otherwise interpreted when
checking their validity.
[0] [llvm-dev] [RFC] DebugInfo: A different way of specifying variable locations post-isel
[1] ⚙ D86812 [DebugInstrRef][1/3] Track PHI values through register allocation
Unfortunately this is expected
Indeed unfortunate, but I understand the reasoning behind it. I've worked with other compilers that attempted to solve the same issue. We ended up making the decision to create hard latencies between the debug instructions and their 'uses', effectively adding the debug into the DAG in a sense. This of course impacts performance, which can be argued to be a greater evil.
my instruction-referencing rewrite of variable locations... might be production ready for llvm 14, but possibly not for VLIW machines
Yes, this sounds like it'd require thought/work on each VLIW backend's part. VLIW can mean different things to different targets and is not very well abstracted at this point. I'll keep an eye on this change!
all register uses in DBG_VALUE / DBG_VALUE_LIST instructions should have the "IsDebug" flag set on them
It looks like the main driver for bundle creation, 'finalizeBundle', doesn't care about that flag at all. The function iterates over all register operands and notes only uses and defs, which get added to the BUNDLE instruction. The only flags that it tracks are dead, kill, and undef. If adding 'debug' to the tracked state fixes our issue, I'll see about putting up a quick patch to get feedback.
Another option is to ensure that debug instructions do not appear 'in' a bundle. When finalizing a bundle, the mechanism could shift all debug instructions to the top, and then only bundle 'real' instructions that come after. Right now, debug instructions are added to the bundle only because they are in between instructions that have been consciously chosen to be in a bundle together. This solves the issue of having to track the debug flag in finalizeBundle.
In any case, thanks much for the response. I have plenty to work off of now.
Regards,
JB
Hi James.
I'm involved in maintaining an out OOT-target, which uses VLIW, and
post-RA scheduling
We never put the DBG-instructions inside the bundles. I don't remember
exactly how much of that is solved by downstream patch-ups of the in-tree
code.
A lot of the backend code see the BUNDLE as one instruction, at least
after it has been finalized. So maybe I also should mention that we
finalize bundles a.s.a.p (already pre-RA if we introduce a bundle
before RA). And since you basically can't break in the debugger in
the middle of a bundle (at least not for our architecture) it doesn't
really make sense to have a DBG-instruction inside the bundle (unless
maybe if the bundle hasn't been finalized and you want to track what
is going on inside the bundle as long as possible).
We also got some extra heuristics/rules related to re-insertion of
DBG-instructions after scheduling. I don't have the exact details
right now, but sometimes we mark the DBG_VALUE as undef when inserting
it. And if for example having something like this
$r0 = load ...
DBG_VALUE $r0, ...
call @func($r0), implicit $r0
and the load and the call is bundled, and $r0 doesn't survive the calls
then I think we just drop the DBG_VALUE, because it would be wrong to
add it after the bundle.
Anyway, I agree that this is a really complicated problem.
I also think our current solution is a bit ad-hoc, just trying to
avoid some simple cases when the scheduling otherwise mess up things.
And at least in the past it has been quite sensitive to placement of
the DBG_VALUE instructions. I.e. if they are close to the definition
of the register that they refer the debug-experience hopefully isn't
that bad. So it isn't something that we have considered to upstream
(it is also probably a bit target dependent in the current state
and I don't know if it would be testable with any in-tree target).
This also reminds be that if we have a DBG_VALUE that refer to a
constant value and not a register, then one can't really look at
register liveness etc to understand if the DBG_VALUE is bogus after
scheduling. A more complete solution would need to consider how a
certain region in the IR maps to the original source, and what value
the variable should have. With WLIV, when we can be at X
different source lines at the same time, that is extra tricky.
I don't remember if there has been any discussions in the past
about allowing/disallowing DGB-instructions inside bundles.
If there would be a rule about not having DBG-instructions inside
bundles, then I think we could try to find any patches we have
downstream related to that and contribute to the upstream main branch.
Regards,
Björn