Skip to content

Commit

Permalink
[DebugInfo][RemoveDIs] Assert if we mix PHIs and debug-info (llvm#84054)
Browse files Browse the repository at this point in the history
A potentially erroneous code construction with the work we've done to
remove debug intrinsics, is inserting PHIs into blocks when the position
hasn't been "sourced correctly". Specifically, if you have:

    %foo = PHI
    #dbg_value
    %bar = add i32...

And plan on inserting a new PHI, you have to use the iterator form of
`getFirstNonPHI` or getFirstInsertionPt (or begin()) to acquire an
iterator that tells the debug-info maintenance code "this is supposed to
be at the start of the block, put it in front of #dbg_value". We can
detect call-sites that aren't doing this at runtime, and should do with
this assertion. It might invalidate code that's doing something very
unexpected, like walking backwards to find a PHI, then going forwards,
then inserting: however that's just an inefficient way of calling
`getFirstNonPHI`.
  • Loading branch information
jmorse authored Mar 11, 2024
1 parent f1aa783 commit c9465e4
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions llvm/lib/IR/Instruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,18 @@ void Instruction::insertBefore(BasicBlock &BB,
if (!InsertAtHead) {
DPMarker *SrcMarker = BB.getMarker(InsertPos);
if (SrcMarker && !SrcMarker->empty()) {
// If this assertion fires, the calling code is about to insert a PHI
// after debug-records, which would form a sequence like:
// %0 = PHI
// #dbg_value
// %1 = PHI
// Which is de-normalised and undesired -- hence the assertion. To avoid
// this, you must insert at that position using an iterator, and it must
// be aquired by calling getFirstNonPHIIt / begin or similar methods on
// the block. This will signal to this behind-the-scenes debug-info
// maintenence code that you intend the PHI to be ahead of everything,
// including any debug-info.
assert(!isa<PHINode>(this) && "Inserting PHI after debug-records!");
adoptDbgValues(&BB, InsertPos, false);
}
}
Expand Down

0 comments on commit c9465e4

Please sign in to comment.