[DebugInfo] Ensure that a demanded bits optimisation in InstCombine does not result in an incorrect debuginfo variable value

[chrisjackson]

Summary

See Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=44371.

A demanded bits optimisation could result in an incorrect debuginfo variable value for a constant operand. This patch attempts to salvage the value before the optimisation is applied.

[Orlando]

LGTM but since we're on the same team (now, woot!) I'd like to give others a chance to have a look.

llvm/test/DebugInfo/X86/instcombine-demanded-bits-salvage.ll

12

As we discussed offline the fact that this dbg.value later becomes undef is unrelated to the behaviour you're checking for (a correct salvage operation) so it'd probably be helpful to either add a comment explaining this, or omit the first dbg.value operand in the CHECK line.

[aprantl]

LGTM but since we're on the same team (now, woot!) I'd like to give others a chance to have a look.

Just pointing out that there's no such requirement, but it never hurts to have someone look at a patch who brings in a different perspective.

I did find a bug in your test :-)
Otherwise this looks good!

llvm/test/DebugInfo/X86/instcombine-demanded-bits-salvage.ll

38

You missed a ':' here!
Also, I would avoid hardcoding the metadata numbers wherever possible. Makes the tests hard to maintain.

[chrisjackson]

Simplified the CHECK expression to match only the component of the dbg.value call that is significant for this test.

[chrisjackson]

Removed unnecessary CHECK line. It was erroneous anyway!

[djtodoro]

LGTM, thanks!

llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp

 bool InstCombiner::SimplifyDemandedBits(Instruction *I, unsigned OpNo,
                                         const APInt &DemandedMask,
                                         KnownBits &Known,
                                         unsigned Depth) {
   Use &U = I->getOperandUse(OpNo);
   Value *NewVal = SimplifyDemandedUseBits(U.get(), DemandedMask, Known,
                                           Depth, I);
   if (!NewVal) return false;
+  // If an operand is modified, ensure we salvage.
+  if (Instruction* OpInst = dyn_cast<Instruction>(U))
+    salvageDebugInfoOrMarkUndef(*OpInst);
   // Add the old operand back to the worklist.
   Worklist.addValue(U.get());
   U = NewVal;
   return true;
 }
 
 
 /// This function attempts to replace V with a simpler value based on the

llvm/test/DebugInfo/X86/instcombine-demanded-bits-salvage.ll

+; RUN: opt -mtriple=x86_64-- %s -S --instcombine  -o - | FileCheck %s
+; Verify that demanded bits optimisations don't affect debuginfo 
+; variable values.
+; Bugzilla #44371
+
+@a = common dso_local local_unnamed_addr global i32 0, align 4
+
+define dso_local i32 @p() local_unnamed_addr !dbg !11 {
+entry:
+  %conv = load i32, i32* @a, align 4, !dbg !14
+  %0 = and i32 %conv, 65535, !dbg !14
+  ; CHECK: call void @llvm.dbg.value(metadata i32 undef, metadata ![[NODE:[0-9]+]], metadata !DIExpression(DW_OP_constu, 65535, DW_OP_and, DW_OP_stack_value))
+  call void @llvm.dbg.value(metadata i32 %0, metadata !15, metadata !DIExpression()), !dbg !14
+  %1 = lshr i32 %0, 12, !dbg !14
+  %2 = and i32 %1, 8, !dbg !14
+  %3 = xor i32 %2, 8, !dbg !14
+  ret i32 0, !dbg !14
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!8, !9}
+!llvm.ident = !{!10}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, globals: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "instcomnine.ll", directory: "/temp/bz44371")
+!2 = !{}
+!3 = !{!7}
+!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!8 = !{i32 7, !"Dwarf Version", i32 4}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{!""}
+!11 = distinct !DISubprogram(name: "p", scope: !1, file: !1, line: 6, type: !12, scopeLine: 6, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!12 = !DISubroutineType(types: !13)
+!13 = !{!7}
+!14 = !DILocation(line: 4, column: 35, scope: !11)
+; CHECK [[NODE]] = !DILocalVariable(name: "p_28", arg: 1, scope: !11, file: !1, line: 16, type: !7)
+!15 = !DILocalVariable(name: "p_28", arg: 1, scope: !11, file: !1, line: 16, type: !7)