Add a DIExpression const-folder to prevent silly expressions

[probinson]

Summary

It's entirely possible (because it actually happened) for a bool
variable to end up with a 256-bit DW_AT_const_value. This came about
when a local bool variable was initialized from a bitfield in a
32-byte struct of bitfields, and after inlining and constant
propagation, the variable did have a constant value. The sequence of
optimizations had it carrying "i256" values around, but once the
constant made it into the llvm.dbg.value, no further IR changes could
affect it.

Technically the llvm.dbg.value did have a DIExpression to reduce it
back down to 8 bits, but the compiler is in no way ready to emit an
oversized constant *and* a DWARF expression to manipulate it.
Depending on the circumstances, we had either just the very fat bool
value, or an expression with no starting value.

The sequence of optimizations that led to this state did seem pretty
reasonable, so the solution I came up with was to invent a DWARF
constant expression folder. Currently it only does convert ops, but
there's no reason it couldn't do other ops if that became useful.

This broke three tests that depended on having convert ops survive
into the DWARF, so I added an operator that would abort the folder to
each of those tests.

[aprantl]

Thanks for looking into this! Is there any advantage to return a Constant+DIExpression, or could we just return a DIExpression(DW_OP_constu, ...)? Or can we not do that because even after folding the constant is > 64bits?
I think this new API would be a great candidate to create a unit test harness for (perhaps in MetadataTest.cpp).

[probinson]

In D106915#2908862, @aprantl wrote:

Thanks for looking into this! Is there any advantage to return a Constant+DIExpression, or could we just return a DIExpression(DW_OP_constu, ...)? Or can we not do that because even after folding the constant is > 64bits?

Looking at the rest of the infrastructure, I think the Constant is not optional, the DIExpression is the optional part. Most of the time there isn't an expression at all, of course, just the initial operand (which might or might not be a Constant). I suppose that could be redesigned but I think it goes beyond the scope of this patch.

I think this new API would be a great candidate to create a unit test harness for (perhaps in MetadataTest.cpp).

You point out something I was thinking after I posted the patch, i.e. that the testing was just the sort I always grump about--it verifies the right thing happens in the case that prompts the patch, without looking at the broader cases. Unit testing will be just the thing.

[StephenTozer]

In D106915#2910494, @probinson wrote:

Looking at the rest of the infrastructure, I think the Constant is not optional, the DIExpression is the optional part. Most of the time there isn't an expression at all, of course, just the initial operand (which might or might not be a Constant). I suppose that could be redesigned but I think it goes beyond the scope of this patch.

I agree - currently we can't support this, but it can go on the list of potential improvements to DIExpressions.

Some boring nits in the test case, but the implementation looks good. It sounds like some more testing is due, but the rest of this patch LGTM.

llvm/test/DebugInfo/X86/DIExpr-const-folding.ll

32

Nit, attribute can be removed.

38–45

Nit, git reference can be removed here.

[probinson]

Added unittest. Addressed review nits.

[probinson]

In D106915#2910725, @StephenTozer wrote:

In D106915#2910494, @probinson wrote:

Looking at the rest of the infrastructure, I think the Constant is not optional, the DIExpression is the optional part. Most of the time there isn't an expression at all, of course, just the initial operand (which might or might not be a Constant). I suppose that could be redesigned but I think it goes beyond the scope of this patch.

I agree - currently we can't support this, but it can go on the list of potential improvements to DIExpressions.

Hmmm currently the folder works on a single known-constant operand; if/when the non-variadic form of the intrinsic/DBG_VALUE is fully replaced by variadic intrinsic/DBG_VALUE_LIST, the operand could become a one-element operand list and we prepend a DW_OP_LLVM_arg to the expression. That would unify how it all looks, I expect. At that point the constant-folder could be modified to take the original operand list and expression (without the caller vetting it as a single constant int) and the folder would do the vetting itself. A minor problem there is that DIExpression (and DebugInfoMetadata.h in general) don't know about operand lists, which appear to be stored in different ways for the IR intrinsic and the SDAG/MachineInstr form. That would have to be unified, presumably within DebugInfoMetadata.

It wouldn't be a stretch, at that point, to say that the folder could import constant operands directly into the expression, and remove them from the operand list. Right now that responsibility lies with the DWARF emitter. We'd also have to look at how the Codeview side works, and make sure we don't break that or make it unreasonably complicated.

[aprantl]

Looking at the rest of the infrastructure, I think the Constant is not optional, the DIExpression is the optional part. Most of the time there isn't an expression at all, of course, just the initial operand (which might or might not be a Constant). I suppose that could be redesigned but I think it goes beyond the scope of this patch.

I see, while we use DW_OP_const for DIGlobalVariableExpression(), there is no canonical way to express a llvm.dbg.value that doesn't bind an SSA value. llvm.dbg.value(undef) has a special meaning, and llvm.dbg.value(null) is treated like undef.

llvm/test/DebugInfo/X86/convert-linked.ll

22–25

This looks wrong: What is a DW_OP_deref of the constant 32 supposed to be?

[aprantl]

llvm/unittests/IR/MetadataTest.cpp

2912

Thanks! Assuming these are dwarf expression ops, could spell them out here using the Dwarf.def enums? Or does that mess with the macro? If so, maybe we could at least add a comment expanding the raw numbers.

[probinson]

llvm/test/DebugInfo/X86/convert-linked.ll

22–25

It is supposed to be a way to keep the constant folder from firing. It doesn't "mean" anything. I can add a comment that says so?

llvm/unittests/IR/MetadataTest.cpp

2912

The values are parameters to the APInt ctor, meaning 32-bits wide with value 117 as input, and 32-bits wide with value 117 as output, and 1 element remaining in the expression afterward. I don't pass the APInt directly because the macro expansion needs to know the bit-width of the result independently. I suppose it could become
EXPECT_EXPR_FOLD(APInt(32, 117), APInt(32, 117), 32, 1)
if that would be easier to read?

[aprantl]

llvm/test/DebugInfo/X86/convert-linked.ll

22–25

I see. A comment explaining the reason for the strange expression would definitely help someone who needs to maintain the testcase later. Maybe we could even use something more neutral like DW_OP_lit0 DW_OP_plus if that also works?

[probinson]

Add commentary to help explain test fragility, and use operators that make more sense.
Rename the helper macro in the unit test to try to make the nature of its arguments clearer.

[probinson]

llvm/unittests/IR/MetadataTest.cpp

2912

Changed the macro name, in case that helps. Let me know if APInt arguments would make it clearer.

llvm/include/llvm/IR/DebugInfoMetadata.h

public:
   }
 
   /// Check if the expression consists of exactly one entry value operand.
   /// (This is the only configuration of entry values that is supported.)
   bool isEntryValue() const {
     return getNumElements() > 0 &&
            getElement(0) == dwarf::DW_OP_LLVM_entry_value;
   }
+
+  /// Try to shorten an expression with an initial constant operand.
+  /// Returns a new expression and constant on success, or the original
+  /// expression and constant on failure.
+  std::pair<DIExpression *, const ConstantInt *>
+  constantFold(const ConstantInt *CI);
 };
 
 inline bool operator==(const DIExpression::FragmentInfo &A,
                        const DIExpression::FragmentInfo &B) {
   return std::tie(A.SizeInBits, A.OffsetInBits) ==
          std::tie(B.SizeInBits, B.OffsetInBits);
 }
 

llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

case Intrinsic::dbg_value: {
     assert(DI->getVariable()->isValidLocationForIntrinsic(DbgLoc) &&
            "Expected inlined-at fields to agree");
     if (!V || isa<UndefValue>(V) || DI->hasArgList()) {
       // DI is either undef or cannot produce a valid DBG_VALUE, so produce an
       // undef DBG_VALUE to terminate any prior location.
       BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II, false, 0U,
               DI->getVariable(), DI->getExpression());
     } else if (const auto *CI = dyn_cast<ConstantInt>(V)) {
+      // See if there's an expression to constant-fold.
+      DIExpression *Expr = DI->getExpression();
+      if (Expr)
+        std::tie(Expr, CI) = Expr->constantFold(CI);
       if (CI->getBitWidth() > 64)
         BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II)
             .addCImm(CI)
             .addImm(0U)
             .addMetadata(DI->getVariable())
-            .addMetadata(DI->getExpression());
+            .addMetadata(Expr);
       else
         BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II)
             .addImm(CI->getZExtValue())
             .addImm(0U)
             .addMetadata(DI->getVariable())
-            .addMetadata(DI->getExpression());
+            .addMetadata(Expr);
     } else if (const auto *CF = dyn_cast<ConstantFP>(V)) {
       BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, II)
           .addFPImm(CF)
           .addImm(0U)
           .addMetadata(DI->getVariable())
           .addMetadata(DI->getExpression());
     } else if (Register Reg = lookUpRegForValue(V)) {
       // FIXME: This does not handle register-indirect values at offset 0.

llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp

MachineInstr *InstrEmitter::EmitDbgNoLocation(SDDbgValue *SD) {
   MIB.addMetadata(Expr);
   return &*MIB;
 }
 
 MachineInstr *
 InstrEmitter::EmitDbgValueFromSingleOp(SDDbgValue *SD,
                                        DenseMap<SDValue, Register> &VRBaseMap) {
   MDNode *Var = SD->getVariable();
-  MDNode *Expr = SD->getExpression();
+  DIExpression *Expr = SD->getExpression();
   DebugLoc DL = SD->getDebugLoc();
   const MCInstrDesc &II = TII->get(TargetOpcode::DBG_VALUE);
 
   assert(SD->getLocationOps().size() == 1 &&
          "Non variadic dbg_value should have only one location op");
 
+  // See about constant-folding the expression.
+  // Copy the location operand in case we replace it.
+  SmallVector<SDDbgOperand, 1> LocationOps(1, SD->getLocationOps()[0]);
+  if (Expr && LocationOps[0].getKind() == SDDbgOperand::CONST) {
+    const Value *V = LocationOps[0].getConst();
+    if (auto *C = dyn_cast<ConstantInt>(V)) {
+      std::tie(Expr, C) = Expr->constantFold(C);
+      LocationOps[0] = SDDbgOperand::fromConst(C);
+    }
+  }
+
   // Emit non-variadic dbg_value nodes as DBG_VALUE.
   // DBG_VALUE := "DBG_VALUE" loc, isIndirect, var, expr
   auto MIB = BuildMI(*MF, DL, II);
-  AddDbgValueLocationOps(MIB, II, SD->getLocationOps(), VRBaseMap);
+  AddDbgValueLocationOps(MIB, II, LocationOps, VRBaseMap);
 
   if (SD->isIndirect())
     MIB.addImm(0U);
   else
     MIB.addReg(0U, RegState::Debug);
 
   return MIB.addMetadata(Var).addMetadata(Expr);
 }

llvm/lib/IR/DebugInfoMetadata.cpp

Optional<DIExpression *> DIExpression::createFragmentExpression(
   }
   assert(Expr && "Unknown DIExpression");
   Ops.push_back(dwarf::DW_OP_LLVM_fragment);
   Ops.push_back(OffsetInBits);
   Ops.push_back(SizeInBits);
   return DIExpression::get(Expr->getContext(), Ops);
 }
 
+std::pair<DIExpression *, const ConstantInt *>
+DIExpression::constantFold(const ConstantInt *CI) {
+  // Copy the APInt so we can modify it.
+  APInt NewInt = CI->getValue();
+  SmallVector<uint64_t, 8> Ops;
+
+  // Fold operators only at the beginning of the expression.
+  bool First = true;
+  bool Changed = false;
+  for (auto Op : expr_ops()) {
+    switch (Op.getOp()) {
+    default:
+      // We fold only the leading part of the expression; if we get to a part
+      // that we're going to copy unchanged, and haven't done any folding,
+      // then the entire expression is unchanged and we can return early.
+      if (!Changed)
+        return {this, CI};
+      First = false;
+      break;
+    case dwarf::DW_OP_LLVM_convert:
+      if (!First)
+        break;
+      Changed = true;
+      if (Op.getArg(1) == dwarf::DW_ATE_signed)
+        NewInt = NewInt.sextOrTrunc(Op.getArg(0));
+      else {
+        assert(Op.getArg(1) == dwarf::DW_ATE_unsigned && "Unexpected operand");
+        NewInt = NewInt.zextOrTrunc(Op.getArg(0));
+      }
+      continue;
+    }
+    Op.appendToVector(Ops);
+  }
+  if (!Changed)
+    return {this, CI};
+  return {DIExpression::get(getContext(), Ops),
+          ConstantInt::get(getContext(), NewInt)};
+}
+
 uint64_t DIExpression::getNumLocationOperands() const {
   uint64_t Result = 0;
   for (auto ExprOp : expr_ops())
     if (ExprOp.getOp() == dwarf::DW_OP_LLVM_arg)
       Result = std::max(Result, ExprOp.getArg(0) + 1);
   assert(hasAllLocationOps(Result) &&
          "Expression is missing one or more location operands.");
   return Result;

llvm/test/DebugInfo/X86/DIExpr-const-folding.ll

+; RUN: llc -mtriple=x86_64 -filetype=obj < %s \
+; RUN:   | llvm-dwarfdump -debug-info - | FileCheck %s
+; RUN: llc -mtriple=x86_64 -filetype=obj -fast-isel < %s \
+; RUN:   | llvm-dwarfdump -debug-info - | FileCheck %s
+
+;; The important thing is the DW_OP_lit2 with no converts.
+;; TODO: Make this work with global isel
+;; Indirectly related FIXME: Should be able to emit DW_AT_const_value instead.
+
+; CHECK: DW_TAG_variable
+; CHECK-NEXT: DW_AT_location (DW_OP_lit2, DW_OP_stack_value)
+; CHECK-NEXT: DW_AT_name ("bIsShowingCollision")
+
+%class.UClient = type { %class.UWorld*, %struct.FFlags }
+%class.UWorld = type { i16 }
+%struct.FFlags = type { [9 x i8], i32 }
+
+define dso_local void @_ZN7UClient13ToggleVolumesEv(%class.UClient* nocapture nonnull align 8 dereferenceable(24) %this) local_unnamed_addr align 2 !dbg !8 {
+entry:
+  call void @llvm.dbg.value(metadata i72 2, metadata !43, metadata !DIExpression(DW_OP_LLVM_convert, 72, DW_ATE_unsigned, DW_OP_LLVM_convert, 8, DW_ATE_unsigned, DW_OP_stack_value)), !dbg !48
+  %World = getelementptr inbounds %class.UClient, %class.UClient* %this, i64 0, i32 0, !dbg !49
+  %0 = load %class.UWorld*, %class.UWorld** %World, align 8, !dbg !49, !tbaa !51
+  %1 = getelementptr %class.UWorld, %class.UWorld* %0, i64 0, i32 0, !dbg !58
+  store i16 2, i16* %1, align 1, !dbg !59
+  ret void, !dbg !60
+}
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 13.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "min-test-3.cpp", directory: "/home/probinson/projects/scratch/tc8251")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 7, !"uwtable", i32 1}
+!7 = !{!"clang version 13.0.0"}
+!8 = distinct !DISubprogram(name: "ToggleVolumes", linkageName: "_ZN7UClient13ToggleVolumesEv", scope: !9, file: !1, line: 39, type: !37, scopeLine: 40, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, declaration: !36, retainedNodes: !40)
+!9 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "UClient", file: !1, line: 31, size: 192, flags: DIFlagTypePassByValue | DIFlagNonTrivial, elements: !10, identifier: "_ZTS7UClient")
+!10 = !{!11, !20, !36}
+!11 = !DIDerivedType(tag: DW_TAG_member, name: "World", scope: !9, file: !1, line: 34, baseType: !12, size: 64, flags: DIFlagPublic)
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !13, size: 64)
+!13 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "UWorld", file: !1, line: 8, size: 16, flags: DIFlagTypePassByValue, elements: !14, identifier: "_ZTS6UWorld")
+!14 = !{!15, !18, !19}
+!15 = !DIDerivedType(tag: DW_TAG_member, name: "bCollision", scope: !13, file: !1, line: 11, baseType: !16, size: 1, flags: DIFlagPublic | DIFlagBitField, extraData: i64 0)
+!16 = !DIDerivedType(tag: DW_TAG_typedef, name: "uint8", file: !1, line: 1, baseType: !17)
+!17 = !DIBasicType(name: "unsigned char", size: 8, encoding: DW_ATE_unsigned_char)
+!18 = !DIDerivedType(tag: DW_TAG_member, name: "dummyA", scope: !13, file: !1, line: 12, baseType: !16, size: 7, offset: 1, flags: DIFlagPublic | DIFlagBitField, extraData: i64 0)
+!19 = !DIDerivedType(tag: DW_TAG_member, name: "dummyB", scope: !13, file: !1, line: 13, baseType: !16, size: 1, offset: 8, flags: DIFlagPublic | DIFlagBitField, extraData: i64 0)
+!20 = !DIDerivedType(tag: DW_TAG_member, name: "EngineShowFlags", scope: !9, file: !1, line: 35, baseType: !21, size: 128, offset: 64, flags: DIFlagPublic)
+!21 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "FFlags", file: !1, line: 16, size: 128, flags: DIFlagTypePassByValue | DIFlagNonTrivial, elements: !22, identifier: "_ZTS6FFlags")
+!22 = !{!23, !26, !27, !28, !29, !30, !32}
+!23 = !DIDerivedType(tag: DW_TAG_member, name: "Volumes", scope: !21, file: !1, line: 18, baseType: !24, size: 1, flags: DIFlagBitField, extraData: i64 0)
+!24 = !DIDerivedType(tag: DW_TAG_typedef, name: "uint32", file: !1, line: 2, baseType: !25)
+!25 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+!26 = !DIDerivedType(tag: DW_TAG_member, name: "Collision", scope: !21, file: !1, line: 19, baseType: !24, size: 1, offset: 1, flags: DIFlagBitField, extraData: i64 0)
+!27 = !DIDerivedType(tag: DW_TAG_member, name: "dummy1", scope: !21, file: !1, line: 20, baseType: !24, size: 30, offset: 2, flags: DIFlagBitField, extraData: i64 0)
+!28 = !DIDerivedType(tag: DW_TAG_member, name: "dummy2", scope: !21, file: !1, line: 21, baseType: !24, size: 32, offset: 32, flags: DIFlagBitField, extraData: i64 0)
+!29 = !DIDerivedType(tag: DW_TAG_member, name: "dummy3", scope: !21, file: !1, line: 22, baseType: !24, size: 1, offset: 64, flags: DIFlagBitField, extraData: i64 0)
+!30 = !DIDerivedType(tag: DW_TAG_member, name: "CustomShowFlags", scope: !21, file: !1, line: 24, baseType: !31, size: 32, offset: 96)
+!31 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!32 = !DISubprogram(name: "FFlags", scope: !21, file: !1, line: 25, type: !33, scopeLine: 25, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized)
+!33 = !DISubroutineType(types: !34)
+!34 = !{null, !35}
+!35 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !21, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!36 = !DISubprogram(name: "ToggleVolumes", linkageName: "_ZN7UClient13ToggleVolumesEv", scope: !9, file: !1, line: 36, type: !37, scopeLine: 36, flags: DIFlagPublic | DIFlagPrototyped, spFlags: DISPFlagOptimized)
+!37 = !DISubroutineType(types: !38)
+!38 = !{null, !39}
+!39 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!40 = !{!41, !43}
+!41 = !DILocalVariable(name: "this", arg: 1, scope: !8, type: !42, flags: DIFlagArtificial | DIFlagObjectPointer)
+!42 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
+!43 = !DILocalVariable(name: "bIsShowingCollision", scope: !44, file: !1, line: 45, type: !46)
+!44 = distinct !DILexicalBlock(scope: !45, file: !1, line: 42, column: 2)
+!45 = distinct !DILexicalBlock(scope: !8, file: !1, line: 41, column: 6)
+!46 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !47)
+!47 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)
+!48 = !DILocation(line: 0, scope: !44)
+!49 = !DILocation(line: 51, column: 8, scope: !50)
+!50 = distinct !DILexicalBlock(scope: !44, file: !1, line: 51, column: 8)
+!51 = !{!52, !53, i64 0}
+!52 = !{!"_ZTS7UClient", !53, i64 0, !56, i64 8}
+!53 = !{!"any pointer", !54, i64 0}
+!54 = !{!"omnipotent char", !55, i64 0}
+!55 = !{!"Simple C++ TBAA"}
+!56 = !{!"_ZTS6FFlags", !57, i64 0, !57, i64 0, !57, i64 0, !57, i64 4, !57, i64 8, !57, i64 12}
+!57 = !{!"int", !54, i64 0}
+!58 = !DILocation(line: 52, column: 12, scope: !50)
+!59 = !DILocation(line: 52, column: 23, scope: !50)
+!60 = !DILocation(line: 55, column: 1, scope: !8)

llvm/test/DebugInfo/X86/convert-debugloc.ll

 ; Test DW_OP_convert + Split DWARF
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -dwarf-version=5 -filetype=obj -O0 < %s -debugger-tune=lldb -dwarf-op-convert=Enable -split-dwarf-file=baz.dwo | llvm-dwarfdump - \
 ; RUN:   | FileCheck %s --check-prefix=CONV --check-prefix=SPLITCONV --check-prefix=SPLIT "--implicit-check-not={{DW_TAG|NULL}}"
 
 ; RUN: llc -mtriple=x86_64 -dwarf-version=5 -filetype=obj -O0 < %s -debugger-tune=gdb  | llvm-dwarfdump -v -debug-info - \
 ; RUN:   | FileCheck %s --check-prefix=VERBOSE --check-prefix=CONV "--implicit-check-not={{DW_TAG|NULL}}"
 
 
-; SPLITCONV: Compile Unit:{{.*}} DWO_id = 0xe91d8d1d7f9782c0
+; SPLITCONV: Compile Unit:{{.*}} DWO_id = 0x62f17241069b1fa3
 ; SPLIT: DW_TAG_skeleton_unit
 
 ; CONV: DW_TAG_compile_unit
 ; CONV:[[SIG8:.*]]:   DW_TAG_base_type
 ; CONV-NEXT:DW_AT_name {{.*}}"DW_ATE_signed_8")
 ; CONV-NEXT:DW_AT_encoding {{.*}}DW_ATE_signed)
 ; CONV-NEXT:DW_AT_byte_size {{.*}}0x01)
 ; CONV-NOT: DW_AT
 ; CONV:[[SIG32:.*]]:   DW_TAG_base_type
 ; CONV-NEXT:DW_AT_name {{.*}}"DW_ATE_signed_32")
 ; CONV-NEXT:DW_AT_encoding {{.*}}DW_ATE_signed)
 ; CONV-NEXT:DW_AT_byte_size {{.*}}0x04)
 ; CONV-NOT: DW_AT
 ; CONV:   DW_TAG_subprogram
 ; CONV:     DW_TAG_formal_parameter
 ; CONV:     DW_TAG_variable
-; CONV:     DW_AT_location {{.*}}DW_OP_constu 0x20, DW_OP_convert (
+; CONV:     DW_AT_location {{.*}}DW_OP_constu 0x20, DW_OP_lit0, DW_OP_plus, DW_OP_convert (
 ; VERBOSE-SAME: [[SIG8]] ->
 ; CONV-SAME: [[SIG8]]) "DW_ATE_signed_8", DW_OP_convert (
 ; VERBOSE-SAME: [[SIG32]] ->
 ; CONV-SAME: [[SIG32]]) "DW_ATE_signed_32", DW_OP_stack_value)
 ; CONV:       DW_AT_name {{.*}}"y")
 ; CONV:     NULL
 ; CONV:   DW_TAG_base_type
 ; CONV:     DW_AT_name {{.*}}"signed char")

 ; NOCONV:     DW_AT_name ("int")
 ; NOCONV:   NULL
 
 
 ; Function Attrs: noinline nounwind uwtable
 define dso_local signext i8 @foo(i8 signext %x) !dbg !7 {
 entry:
   call void @llvm.dbg.value(metadata i8 %x, metadata !11, metadata !DIExpression()), !dbg !12
-  call void @llvm.dbg.value(metadata i8 32, metadata !13, metadata !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)), !dbg !15
+;; This test depends on "convert" surviving all the way to the final object.
+;; So, insert something before DW_OP_LLVM_convert that the expression folder
+;; will not attempt to eliminate.  At the moment, only "convert" ops are folded.
+;; If you have to change the expression, the expected DWO_id also changes.
+  call void @llvm.dbg.value(metadata i8 32, metadata !13, metadata !DIExpression(DW_OP_lit0, DW_OP_plus, DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)), !dbg !15
   ret i8 %x, !dbg !16
 }
 
 ; Function Attrs: nounwind readnone speculatable
 declare void @llvm.dbg.declare(metadata, metadata, metadata)
 
 ; Function Attrs: nounwind readnone speculatable
 declare void @llvm.dbg.value(metadata, metadata, metadata)

llvm/test/DebugInfo/X86/convert-linked.ll

 ; CHECK: [[CU1BT1:0x[0-9a-f]+]]: DW_TAG_base_type
 ; CHECK-NEXT: DW_ATE_signed_16
 ; CHECK: DW_TAG_variable
 ; CHECK: DW_OP_convert ([[CU1BT0]]) "DW_ATE_signed_8", DW_OP_convert ([[CU1BT1]]) "DW_ATE_signed_16"
 
 define dso_local signext i8 @foo(i8 signext %x) !dbg !9 {
 entry:
   call void @llvm.dbg.value(metadata i8 %x, metadata !13, metadata !DIExpression()), !dbg !14
-  call void @llvm.dbg.value(metadata i8 32, metadata !15, metadata !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)), !dbg !17
+;; This test depends on "convert" surviving all the way to the final object.
+;; So, insert something before DW_OP_LLVM_convert that the expression folder
+;; will not attempt to eliminate.  At the moment, only "convert" ops are folded.
+  call void @llvm.dbg.value(metadata i8 32, metadata !15, metadata !DIExpression(DW_OP_lit0, DW_OP_plus, DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 32, DW_ATE_signed, DW_OP_stack_value)), !dbg !17
   ret i8 %x, !dbg !18
 }
 
 define dso_local signext i8 @bar(i8 signext %x) !dbg !19 {
 entry:
   call void @llvm.dbg.value(metadata i8 %x, metadata !20, metadata !DIExpression()), !dbg !21
-  call void @llvm.dbg.value(metadata i8 32, metadata !22, metadata !DIExpression(DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 16, DW_ATE_signed, DW_OP_stack_value)), !dbg !24
+;; This test depends on "convert" surviving all the way to the final object.
+;; So, insert something before DW_OP_LLVM_convert that the expression folder
+;; will not attempt to eliminate.  At the moment, only "convert" ops are folded.
+  call void @llvm.dbg.value(metadata i8 32, metadata !22, metadata !DIExpression(DW_OP_lit0, DW_OP_plus, DW_OP_LLVM_convert, 8, DW_ATE_signed, DW_OP_LLVM_convert, 16, DW_ATE_signed, DW_OP_stack_value)), !dbg !24
   ret i8 %x, !dbg !25
 }
 
 ; Function Attrs: nounwind readnone speculatable
 declare void @llvm.dbg.value(metadata, metadata, metadata) #1
 
 attributes #1 = { nounwind readnone speculatable }
 

llvm/test/DebugInfo/X86/convert-loclist.ll

 ; RUN: llc -mtriple=x86_64 -filetype=obj < %s \
 ; RUN:   | llvm-dwarfdump -debug-info -debug-loclists - | FileCheck %s
 ; RUN: llc -mtriple=x86_64 -split-dwarf-file=foo.dwo -filetype=obj -dwarf-op-convert=Enable < %s \
 ; RUN:   | llvm-dwarfdump -debug-info -debug-loclists - | FileCheck --check-prefix=SPLIT --check-prefix=CHECK %s
 ; RUN: llc -mtriple=x86_64 -split-dwarf-file=foo.dwo -filetype=asm -dwarf-op-convert=Enable < %s \
 ; RUN:   | FileCheck --check-prefix=ASM %s
 
 ; A bit of a brittle test - this is testing the specific DWO_id. The
 ; alternative would be to test two files with different DW_OP_convert values &
 ; ensuring the DWO IDs differ when the DW_OP_convert parameter differs.
 
 ; So if this test ends up being a brittle pain to maintain, updating the DWO ID
 ; often - add another IR file with a different DW_OP_convert that's otherwise
 ; identical and demonstrate that they have different DWO IDs.
 
-; SPLIT: 0x00000000: Compile Unit: {{.*}} DWO_id = 0xafd73565c68bc661
+; SPLIT: 0x00000000: Compile Unit: {{.*}} DWO_id = 0xecf2563326b0bdd3
 
 ; Regression testing a fairly quirky bug where instead of hashing (see above),
 ; extra bytes would be emitted into the output assembly in no
 ; particular/intentional section - so let's check they don't show up at all:
 ; ASM-NOT: .asciz  "\200\200\200"
 
 ; CHECK: 0x{{0*}}[[TYPE:.*]]: DW_TAG_base_type
 ; CHECK-NEXT:                   DW_AT_name ("DW_ATE_unsigned_32")
 
-; CHECK: DW_LLE_offset_pair ({{.*}}): DW_OP_consts +7, DW_OP_convert 0x[[TYPE]], DW_OP_stack_value
+; CHECK: DW_LLE_offset_pair ({{.*}}): DW_OP_consts +7, DW_OP_lit0, DW_OP_plus, DW_OP_convert 0x[[TYPE]], DW_OP_stack_value
 
 ; Function Attrs: uwtable
 define dso_local void @_Z2f2v() local_unnamed_addr #0 !dbg !11 {
 entry:
   tail call void @_Z2f1v(), !dbg !15
-  call void @llvm.dbg.value(metadata i32 7, metadata !13, metadata !DIExpression(DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value)), !dbg !16
+;; This test depends on "convert" surviving all the way to the final object.
+;; So, insert something before DW_OP_LLVM_convert that the expression folder
+;; will not attempt to eliminate.  At the moment, only "convert" ops are folded.
+;; If you have to change the expression, the expected DWO_id also changes.
+  call void @llvm.dbg.value(metadata i32 7, metadata !13, metadata !DIExpression(DW_OP_lit0, DW_OP_plus, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value)), !dbg !16
   tail call void @_Z2f1v(), !dbg !17
   ret void, !dbg !18
 }
 
 declare !dbg !4 dso_local void @_Z2f1v() local_unnamed_addr #1
 
 ; Function Attrs: nounwind readnone speculatable willreturn
 declare void @llvm.dbg.value(metadata, metadata, metadata) #2

llvm/unittests/IR/MetadataTest.cpp

EXPECT_REPLACE_ARG_EQ(N, 2, 0, dwarf::DW_OP_LLVM_arg, 0,
                         dwarf::DW_OP_LLVM_arg, 0, dwarf::DW_OP_mul);
   EXPECT_REPLACE_ARG_EQ(N, 2, 1, dwarf::DW_OP_LLVM_arg, 0,
                         dwarf::DW_OP_LLVM_arg, 1, dwarf::DW_OP_plus,
                         dwarf::DW_OP_LLVM_arg, 1, dwarf::DW_OP_mul);
 
 #undef EXPECT_REPLACE_ARG_EQ
 }
 
+TEST_F(DIExpressionTest, foldConstant) {
+  const ConstantInt *Int;
+  const ConstantInt *NewInt;
+  DIExpression *Expr;
+  DIExpression *NewExpr;
+
+#define EXPECT_FOLD_CONST(StartWidth, StartValue, EndWidth, EndValue, NumElts)  \
+  Int = ConstantInt::get(Context, APInt(StartWidth, StartValue));               \
+  std::tie(NewExpr, NewInt) = Expr->constantFold(Int);                          \
+  ASSERT_EQ(NewInt->getBitWidth(), EndWidth##u);                                \
+  EXPECT_EQ(NewInt->getValue(), APInt(EndWidth, EndValue));                     \
+  EXPECT_EQ(NewExpr->getNumElements(), NumElts##u)
+
+  // Unfoldable expression should return the original unmodified Int/Expr.
+  Expr = DIExpression::get(Context, {dwarf::DW_OP_deref});
+  EXPECT_FOLD_CONST(32, 117, 32, 117, 1);
+  EXPECT_EQ(NewExpr, Expr);
+  EXPECT_EQ(NewInt, Int);
+  EXPECT_TRUE(NewExpr->startsWithDeref());
+
+  // One unsigned bit-width conversion.
+  Expr = DIExpression::get(
+      Context, {dwarf::DW_OP_LLVM_convert, 72, dwarf::DW_ATE_unsigned});
+  EXPECT_FOLD_CONST(8, 12, 72, 12, 0);
+
+  // Two unsigned bit-width conversions (mask truncation).
+  Expr = DIExpression::get(
+      Context, {dwarf::DW_OP_LLVM_convert, 8, dwarf::DW_ATE_unsigned,
+                dwarf::DW_OP_LLVM_convert, 16, dwarf::DW_ATE_unsigned});
+  EXPECT_FOLD_CONST(32, -1, 16, 0xff, 0);
+
+  // Sign extension.
+  Expr = DIExpression::get(
+      Context, {dwarf::DW_OP_LLVM_convert, 32, dwarf::DW_ATE_signed});
+  EXPECT_FOLD_CONST(16, -1, 32, -1, 0);
+
+  // Get non-foldable operations back in the new Expr.
+  uint64_t Elements[] = {dwarf::DW_OP_deref, dwarf::DW_OP_stack_value};
+  ArrayRef<uint64_t> Expected = Elements;
+  Expr = DIExpression::get(
+      Context, {dwarf::DW_OP_LLVM_convert, 32, dwarf::DW_ATE_signed});
+  Expr = DIExpression::append(Expr, Expected);
+  ASSERT_EQ(Expr->getNumElements(), 5u);
+  EXPECT_FOLD_CONST(16, -1, 32, -1, 2);
+  EXPECT_EQ(NewExpr->getElements(), Expected);
+
+#undef EXPECT_FOLD_CONST
+}
+
 typedef MetadataTest DIObjCPropertyTest;
 
 TEST_F(DIObjCPropertyTest, get) {
   StringRef Name = "name";
   DIFile *File = getFile();
   unsigned Line = 5;
   StringRef GetterName = "getter";
   StringRef SetterName = "setter";