[Utils] Salvage debug info from dead 'or' instructions

[ibadawi]

Summary

Extend salvageDebugInfo to preserve the debug info from a dead 'or' with a constant.

[aprantl]

lib/Transforms/Utils/Local.cpp

1545

That codepath doesn't look like it's tested?

1548

Could any of the other cases be refactored to make use of this new helper, too?

[ibadawi]

lib/Transforms/Utils/Local.cpp

1545

This is the original code path, covered by test/Transforms/InstCombine/debuginfo_add.ll.

1548

I considered also extracting the load case below, but it made things a bit messy since it's not a stack value, so I left it alone.

[aprantl]

Right. Thanks!

[davide]

LGTM if Adrian/Vedant are happy with this.

[ibadawi]

Thanks for the review. I don't have commit access, so can one of you commit it for me? Thanks.

[vsk]

Thank you @ibadawi, this looks great. I can take care of committing this for you.

include/llvm/IR/DebugInfoMetadata.h

public:
   enum { NoDeref = false, WithDeref = true, WithStackValue = true };
 
   /// Prepend \p DIExpr with a deref and offset operation and optionally turn it
   /// into a stack value.
   static DIExpression *prepend(const DIExpression *DIExpr, bool DerefBefore,
                                int64_t Offset = 0, bool DerefAfter = false,
                                bool StackValue = false);
 
+  /// Prepend \p DIExpr with the given opcodes and optionally turn it into a
+  /// stack value.
+  static DIExpression *doPrepend(const DIExpression *DIExpr,
+                                 SmallVectorImpl<uint64_t> &Ops,
+                                 bool StackValue = false);
+
   /// Create a DIExpression to describe one part of an aggregate variable that
   /// is fragmented across multiple Values. The DW_OP_LLVM_fragment operation
   /// will be appended to the elements of \c Expr. If \c Expr already contains
   /// a \c DW_OP_LLVM_fragment \c OffsetInBits is interpreted as an offset
   /// into the existing fragment.
   ///
   /// \param OffsetInBits Offset of the piece in bits.
   /// \param SizeInBits   Size of the piece in bits.

lib/CodeGen/AsmPrinter/DwarfExpression.cpp

while (ExprCursor) {
     case dwarf::DW_OP_plus_uconst:
       assert(LocationKind != Register);
       emitOp(dwarf::DW_OP_plus_uconst);
       emitUnsigned(Op->getArg(0));
       break;
     case dwarf::DW_OP_plus:
     case dwarf::DW_OP_minus:
     case dwarf::DW_OP_mul:
+    case dwarf::DW_OP_or:
       emitOp(Op->getOp());
       break;
     case dwarf::DW_OP_deref:
       assert(LocationKind != Register);
       if (LocationKind != Memory && isMemoryLocation(ExprCursor))
         // Turning this into a memory location description makes the deref
         // implicit.
         LocationKind = Memory;

lib/IR/DebugInfoMetadata.cpp

case dwarf::DW_OP_swap: {
         return false;
       break;
     }
     case dwarf::DW_OP_constu:
     case dwarf::DW_OP_plus_uconst:
     case dwarf::DW_OP_plus:
     case dwarf::DW_OP_minus:
     case dwarf::DW_OP_mul:
+    case dwarf::DW_OP_or:
     case dwarf::DW_OP_deref:
     case dwarf::DW_OP_xderef:
       break;
     }
   }
   return true;
 }
 
DIExpression *DIExpression::prepend(const DIExpression *Expr, bool DerefBefore,
   SmallVector<uint64_t, 8> Ops;
   if (DerefBefore)
     Ops.push_back(dwarf::DW_OP_deref);
   
   appendOffset(Ops, Offset);
   if (DerefAfter)
     Ops.push_back(dwarf::DW_OP_deref);
 
+  return doPrepend(Expr, Ops, StackValue);
+}
+
+DIExpression *DIExpression::doPrepend(const DIExpression *Expr,
+                                      SmallVectorImpl<uint64_t> &Ops,
+                                      bool StackValue) {
   if (Expr)
     for (auto Op : Expr->expr_ops()) {
       // A DW_OP_stack_value comes at the end, but before a DW_OP_LLVM_fragment.
       if (StackValue) {
         if (Op.getOp() == dwarf::DW_OP_stack_value)
           StackValue = false;
         else if (Op.getOp() == dwarf::DW_OP_LLVM_fragment) {
           Ops.push_back(dwarf::DW_OP_stack_value);

lib/Transforms/Utils/Local.cpp

if (DbgUsers.empty())
     return;
 
   auto &M = *I.getModule();
 
   auto wrapMD = [&](Value *V) {
     return MetadataAsValue::get(I.getContext(), ValueAsMetadata::get(V));
   };
 
-  auto applyOffset = [&](DbgInfoIntrinsic *DII, uint64_t Offset) {
+  auto doSalvage = [&](DbgInfoIntrinsic *DII, SmallVectorImpl<uint64_t> &Ops) {
     auto *DIExpr = DII->getExpression();
-    DIExpr = DIExpression::prepend(DIExpr, DIExpression::NoDeref, Offset,
-                                   DIExpression::NoDeref,
+    DIExpr = DIExpression::doPrepend(DIExpr, Ops,
                                      DIExpression::WithStackValue);
     DII->setOperand(0, wrapMD(I.getOperand(0)));
     DII->setOperand(2, MetadataAsValue::get(I.getContext(), DIExpr));
     DEBUG(dbgs() << "SALVAGE: " << *DII << '\n');
   };
 
+  auto applyOffset = [&](DbgInfoIntrinsic *DII, uint64_t Offset) {
+    SmallVector<uint64_t, 8> Ops;
+    DIExpression::appendOffset(Ops, Offset);
+    doSalvage(DII, Ops);
+  };
+
+  auto applyOps = [&](DbgInfoIntrinsic *DII,
+                      std::initializer_list<uint64_t> Opcodes) {
+    SmallVector<uint64_t, 8> Ops(Opcodes);
+    doSalvage(DII, Ops);
+  };
+
   if (isa<BitCastInst>(&I) || isa<IntToPtrInst>(&I)) {
     // Bitcasts are entirely irrelevant for debug info. Rewrite dbg.value,
     // dbg.addr, and dbg.declare to use the cast's source.
     for (auto *DII : DbgUsers) {
       DII->setOperand(0, wrapMD(I.getOperand(0)));
       DEBUG(dbgs() << "SALVAGE: " << *DII << '\n');
     }
   } else if (auto *GEP = dyn_cast<GetElementPtrInst>(&I)) {
     unsigned BitWidth =
         M.getDataLayout().getPointerSizeInBits(GEP->getPointerAddressSpace());
     // Rewrite a constant GEP into a DIExpression.  Since we are performing
     // arithmetic to compute the variable's *value* in the DIExpression, we
     // need to mark the expression with a DW_OP_stack_value.
     APInt Offset(BitWidth, 0);
     if (GEP->accumulateConstantOffset(M.getDataLayout(), Offset))
       for (auto *DII : DbgUsers)
         applyOffset(DII, Offset.getSExtValue());
   } else if (auto *BI = dyn_cast<BinaryOperator>(&I)) {
-    if (BI->getOpcode() == Instruction::Add)
+    if (BI->getOpcode() == Instruction::Add ||
+        BI->getOpcode() == Instruction::Or)
       if (auto *ConstInt = dyn_cast<ConstantInt>(I.getOperand(1)))
-        if (ConstInt->getBitWidth() <= 64)
+        if (ConstInt->getBitWidth() <= 64) {
+          uint64_t Val = ConstInt->getSExtValue();
           for (auto *DII : DbgUsers)
-            applyOffset(DII, ConstInt->getSExtValue());
+            if (BI->getOpcode() == Instruction::Add)
+              applyOffset(DII, Val);
+            else if (BI->getOpcode() == Instruction::Or)
+              applyOps(DII, {dwarf::DW_OP_constu, Val, dwarf::DW_OP_or});
+        }
   } else if (isa<LoadInst>(&I)) {
     MetadataAsValue *AddrMD = wrapMD(I.getOperand(0));
     for (auto *DII : DbgUsers) {
       // Rewrite the load into DW_OP_deref.
       auto *DIExpr = DII->getExpression();
       DIExpr = DIExpression::prepend(DIExpr, DIExpression::WithDeref);
       DII->setOperand(0, AddrMD);
       DII->setOperand(2, MetadataAsValue::get(I.getContext(), DIExpr));

test/Transforms/InstCombine/debuginfo-dce-or.ll

+; RUN: opt -instcombine %s -o - -S | FileCheck %s
+
+source_filename = "test.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux"
+
+define void @salvage_or(i64 %context) !dbg !9 {
+entry:
+  %0 = or i64 %context, 256, !dbg !18
+; CHECK:  call void @llvm.dbg.value(metadata i64 %context,
+; CHECK-SAME:                       metadata !DIExpression(DW_OP_constu, 256, DW_OP_or, DW_OP_stack_value))
+  call void @llvm.dbg.value(metadata i64 %0, metadata !14, metadata !16), !dbg !19
+  ret void, !dbg !20
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+
+attributes #0 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!5, !6, !7}
+!llvm.ident = !{!8}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 7.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3)
+!1 = !DIFile(filename: "test.c", directory: "/")
+!2 = !{}
+!3 = !{}
+!4 = !DIBasicType(name: "long long int", size: 64, encoding: DW_ATE_signed)
+!5 = !{i32 2, !"Dwarf Version", i32 4}
+!6 = !{i32 2, !"Debug Info Version", i32 3}
+!7 = !{i32 1, !"PIC Level", i32 2}
+!8 = !{!"clang version 7.0.0"}
+!9 = distinct !DISubprogram(name: "salvage_or", scope: !1, file: !1, line: 91, type: !10, isLocal: false, isDefinition: true, scopeLine: 93, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !12)
+!10 = !DISubroutineType(types: !11)
+!11 = !{!4}
+!12 = !{!13, !14}
+!13 = !DILocalVariable(name: "context", arg: 2, scope: !9, file: !1, line: 92, type: !4)
+!14 = !DILocalVariable(name: "master", scope: !9, file: !1, line: 94, type: !4)
+!15 = !{}
+!16 = !DIExpression()
+!17 = !DILexicalBlockFile(scope: !9, file: !1, discriminator: 2)
+!18 = !DILocation(line: 94, column: 18, scope: !17)
+!19 = !DILocation(line: 94, column: 18, scope: !9)
+!20 = !DILocation(line: 126, column: 1, scope: !9)