[InstCombine][DebugInfo] Fold constants wrapped in metadata

[dstenb]

Summary

When constant folding, constants that are wrapped in metadata were not
folded. This could lead to dbg.values being the only user of a constant
expression, due to the non-dbg uses having been rewritten, resulting in
the constant later on being removed by some other pass. This occurred
with the attached test case, in which the non-rewritten GEP in the
dbg.value intrinsic was later on removed by globalopt.

This patch makes the code look through metadata and fold such constants.

I guess that we in the future may want to allow dbg.values using GEPs and
other constant expressions to be emittable even if there are no non-dbg
uses, but for example SelectionDAG does not support that.

[aprantl]

I think this makes sense. Thanks!

[vsk]

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

3571

I think this is the right way to implement this change at the moment. If a similar need comes up in other contexts, we might consider adding a helper API to Use e.g. std::pair<Value *, bool /* IsMetadataAsValue */> Use::getUnderlyingValue().

llvm/test/Transforms/InstCombine/constant-fold-metadata-wrapped.ll

16

Could you mention that the constant fold here is to turn an i32 GEP operand into i64? It's slightly tricky to spot.

[davide]

LGTM with Vedant's comment addressed.

[dstenb]

Address review comment.

[dstenb]

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

3571

That sounds like a good future improvement! I now skimmed through the MetadataAsValue uses in lib/ to find some similar code pattern, but I did not find any.

[dstenb]

I reverted this in 982944525c7706c4dee00042d5b7cf2f0d87804f. Downstream we encountered debug-invariance (different code with/without -g), which seems to be due to a pre-existing issue in GlobalOpt. I'll find a good upstream reproducer and write a PR for that.

EDIT: I want to clarify that I have been able to reproduce this upstream with an x86 reproducer; I just need to reduce the reproducer.

[dstenb]

In D73630#1867410, @dstenb wrote:

I reverted this in 982944525c7706c4dee00042d5b7cf2f0d87804f. Downstream we encountered debug-invariance (different code with/without -g), which seems to be due to a pre-existing issue in GlobalOpt. I'll find a good upstream reproducer and write a PR for that.

I wrote https://bugs.llvm.org/show_bug.cgi?id=44880 for that.

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

for (BasicBlock::iterator BBI = BB->begin(), E = BB->end(); BBI != E; ) {
           if (isInstructionTriviallyDead(Inst, TLI))
             Inst->eraseFromParent();
           MadeIRChange = true;
           continue;
         }
 
       // See if we can constant fold its operands.
       for (Use &U : Inst->operands()) {
-        if (!isa<ConstantVector>(U) && !isa<ConstantExpr>(U))
+        bool WrapAsMetadata = false;
+        auto *V = cast<Value>(U);
+
+        // Look through metadata wrappers.
+        if (auto *MAV = dyn_cast<MetadataAsValue>(V))
+          if (auto *VAM = dyn_cast<ValueAsMetadata>(MAV->getMetadata())) {
+            V = VAM->getValue();
+            WrapAsMetadata = true;
+          }
+
+        if (!isa<ConstantVector>(V) && !isa<ConstantExpr>(V))
           continue;
 
-        auto *C = cast<Constant>(U);
+        auto *C = cast<Constant>(V);
         Constant *&FoldRes = FoldedConstants[C];
         if (!FoldRes)
           FoldRes = ConstantFoldConstant(C, DL, TLI);
         if (!FoldRes)
           FoldRes = C;
 
         if (FoldRes != C) {
           LLVM_DEBUG(dbgs() << "IC: ConstFold operand of: " << *Inst
                             << "\n    Old = " << *C
                             << "\n    New = " << *FoldRes << '\n');
+          if (WrapAsMetadata)
+            U = MetadataAsValue::get(Inst->getContext(),
+                                     ValueAsMetadata::get(FoldRes));
+          else
             U = FoldRes;
           MadeIRChange = true;
         }
       }
 
       // Skip processing debug intrinsics in InstCombine. Processing these call instructions
       // consumes non-trivial amount of time and provides no value for the optimization.
       if (!isa<DbgInfoIntrinsic>(Inst))
         InstrsForInstCombineWorklist.push_back(Inst);

llvm/test/Transforms/InstCombine/constant-fold-metadata-wrapped.ll

+; RUN: opt -S -instcombine %s -o - | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.Foo = type { i32 }
+
+@global = common global %struct.Foo zeroinitializer, align 4
+
+; Verify that we constant fold the GEP in the llvm.dbg.value intrinsic. We
+; want it to match with the non-dbg GEP, so that the debug information can be
+; emitted later on.
+
+; When constant folding the GEP the index operand types are canonicalized, so
+; we get an i64 operand here.
+; CHECK: call void @llvm.dbg.value(metadata i32* getelementptr inbounds (%struct.Foo, %struct.Foo* @global, i64 0, i32 0)
+; CHECK: call void @ext(i32* getelementptr inbounds (%struct.Foo, %struct.Foo* @global, i64 0, i32 0))
+
+; Function Attrs: nounwind uwtable
+define i32 @main() #0 !dbg !13 {
+entry:
+  call void @llvm.dbg.value(metadata i32* getelementptr inbounds (%struct.Foo, %struct.Foo* @global, i32 0, i32 0), metadata !17, metadata !DIExpression()), !dbg !18
+  call void @ext(i32* getelementptr inbounds (%struct.Foo, %struct.Foo* @global, i32 0, i32 0)), !dbg !19
+  ret i32 0, !dbg !20
+}
+
+declare !dbg !4 void @ext(i32*)
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind uwtable }
+attributes #1 = { nounwind readnone speculatable willreturn }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!9, !10, !11}
+!llvm.ident = !{!12}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, globals: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "foo.c", directory: "/")
+!2 = !{}
+!3 = !{!4}
+!4 = !DISubprogram(name: "ext", scope: !1, file: !1, line: 3, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+!5 = !DISubroutineType(types: !6)
+!6 = !{null, !7}
+!7 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !8, size: 64)
+!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!9 = !{i32 7, !"Dwarf Version", i32 4}
+!10 = !{i32 2, !"Debug Info Version", i32 3}
+!11 = !{i32 1, !"wchar_size", i32 4}
+!12 = !{!"clang version 11.0.0 "}
+!13 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 5, type: !14, scopeLine: 5, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !16)
+!14 = !DISubroutineType(types: !15)
+!15 = !{!8}
+!16 = !{!17}
+!17 = !DILocalVariable(name: "local", scope: !13, file: !1, line: 6, type: !7)
+!18 = !DILocation(line: 0, scope: !13)
+!19 = !DILocation(line: 7, scope: !13)
+!20 = !DILocation(line: 8, scope: !13)