From c9465e4771c93adfbc99ffca5963a48a5334d98d Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Mon, 11 Mar 2024 08:58:59 +0000 Subject: [PATCH] [DebugInfo][RemoveDIs] Assert if we mix PHIs and debug-info (#84054) 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`. --- llvm/lib/IR/Instruction.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp index ce221758ef798b..e863ef3eb8d6d7 100644 --- a/llvm/lib/IR/Instruction.cpp +++ b/llvm/lib/IR/Instruction.cpp @@ -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(this) && "Inserting PHI after debug-records!"); adoptDbgValues(&BB, InsertPos, false); } }