Improve the effectiveness of BDCE's debug info salvaging

[aprantl]

Summary

This patch improves the effectiveness of BDCE's debug info salvaging by processing the instructions in reverse order and delaying dropAllReferences until after debug info salvaging. This allows salvaging of entire chains of deleted instructions!

Previously we would remove all references from an instruction, which would make it impossible to use that instruction to salvage a later instruction in the instruction stream, because its operands were already removed.

See also https://reviews.llvm.org/D110462

[djtodoro]

lgtm, thanks!

[nikic]

I've reverted this in https://github.com/llvm/llvm-project/commit/abbbc480a152c1afbac215064f22ae07c87ed964 because it causes an assertion failure during the test-suite build (using O3.cmake).

[aprantl]

In D110568#3028412, @nikic wrote:

I've reverted this in https://github.com/llvm/llvm-project/commit/abbbc480a152c1afbac215064f22ae07c87ed964 because it causes an assertion failure during the test-suite build (using O3.cmake).

Thanks! It looks like it also triggered the asan bot. I'll take a look.

llvm/lib/Transforms/Scalar/BDCE.cpp

if (I.mayHaveSideEffects() && I.use_empty())
       continue;
 
     // Remove instructions that are dead, either because they were not reached
     // during analysis or have no demanded bits.
     if (DB.isInstructionDead(&I) ||
         (I.getType()->isIntOrIntVectorTy() &&
          DB.getDemandedBits(&I).isNullValue() &&
          wouldInstructionBeTriviallyDead(&I))) {
-      salvageDebugInfo(I);
       Worklist.push_back(&I);
-      I.dropAllReferences();
       Changed = true;
       continue;
     }
 
     // Convert SExt into ZExt if none of the extension bits is required
     if (SExtInst *SE = dyn_cast<SExtInst>(&I)) {
       APInt Demanded = DB.getDemandedBits(SE);
       const uint32_t SrcBitSize = SE->getSrcTy()->getScalarSizeInBits();
for (Use &U : I.operands()) {
       // This should be reconsidered once we settle on the semantics of
       // undef, poison, etc.
       U.set(ConstantInt::get(U->getType(), 0));
       ++NumSimplified;
       Changed = true;
     }
   }
 
+  for (Instruction *&I : llvm::reverse(Worklist)) {
+    salvageDebugInfo(*I);
+    I->dropAllReferences();
+  }
+
   for (Instruction *&I : Worklist) {
     ++NumRemoved;
     I->eraseFromParent();
   }
 
   return Changed;
 }
 

llvm/test/Transforms/Util/salvage-debuginfo.ll

 ; RUN: opt -adce %s -S -o - | FileCheck %s
+; RUN: opt -bdce %s -S -o - | FileCheck %s
 target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx"
 define void @f(i32) !dbg !8 {
 entry:
   %p_x = inttoptr i32 %0 to i8*
   %i_x = ptrtoint i8* %p_x to i32
   ; CHECK: call void @llvm.dbg.value(metadata i32 %0,
   ; CHECK-SAME: !DIExpression(DW_OP_LLVM_convert, 32, DW_ATE_unsigned,