================
@@ -1129,13 +1130,17 @@ static void
cloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
Instruction *NewBonusInst = BonusInst.clone();
- if (!isa<DbgInfoIntrinsic>(BonusInst) &&
- PTI->getDebugLoc() != NewBonusInst->getDebugLoc()) {
- // Unless the instruction has the same !dbg location as the original
- // branch, drop it. When we fold the bonus instructions we want to make
- // sure we reset their debug locations in order to avoid stepping on
- // dead code caused by folding dead branches.
- NewBonusInst->setDebugLoc(DebugLoc());
+ if (!isa<DbgInfoIntrinsic>(BonusInst)) {
+ if (!NewBonusInst->getDebugLoc().isSameSourceLocation(
+ PTI->getDebugLoc())) {
+ // Unless the instruction has the same !dbg location as the original
+ // branch, drop it. When we fold the bonus instructions we want to make
+ // sure we reset their debug locations in order to avoid stepping on
+ // dead code caused by folding dead branches.
+ NewBonusInst->setDebugLoc(DebugLoc());
+ } else if (const DebugLoc &DL = NewBonusInst->getDebugLoc()) {
+ mapAtomInstance(DL, VMap);
----------------
jmorse wrote:
For the first part I think that answers the question (multiple instructions
with the same source-location getting is_stmt). Would there also be potential
for an intervening non-is_stmt instruction with the same source line between
the two? It feels possible from instruction scheduling if not immediately in
this pass, and if so I guess it's a general consequence of the technique.
It doesn't seem unreasonable, although perhaps we need some (debuggers) test
coverage of how to interpret it. After all we're not trying to program the
debugger from the compiler, we're communicating that "this source location
really does happen to execute twice".
> I'm not sure what you mean about PTI's location becoming key? e.g. looking at
> the test - the new branches in b and c are only "key" because they're clones
> of the cond br from d (which is already "key").
I was wondering whether there can be a case where the instruction at PTI and
the bonus instruction are in the same group, but the one being remapped here
has the higher rank, and putting it in a different group causes the
previously-lower-ranked instruction to become the highest ranked as a result.
At face value this isn't a problem because the whole point of this function is
we're duplicating a code path; but could there be scenarios where LLVM uses
this utility to implement a move (i.e. clone-then-delete-old-path?)
Or to put it another way: I believe (but may be wrong) that
`cloneInstructionsIntoPredecessorBlockAndUpdateSSAUses` currently copies/moves
instructions from one place to another. But, with this change, what was a plain
copy/move now comes with changes to the stepping behaviour.
I think this boils down to saying "so what?", and the answer to that is "the
stepping is still superior to without key instructions". I'm just trying to
think through the consequences of how these changes compose together.
https://github.com/llvm/llvm-project/pull/133482
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
