Disallow shift operations in debug expressions spanning multiple registers

[StephenTozer]

Summary

Split from another patch, which this patch should precede: https://reviews.llvm.org/D61184

This patch fixes an issue which was exposed in the initial merge of the patch in the review linked above, in which:

• Debug expressions containing shift operators could be split across multiple registers during instruction selection, resulting in incorrect debug information as the shifted bits will not carry across these registers.
• When a debug expression is split into fragments during instruction selection and the expression cannot be made into a valid fragment expression then no DBG_VALUE instruction is produced; this patch correctly adds an undef DBG_VALUE instruction, preventing the debug info from becoming outdated.

[vsk]

llvm/test/CodeGen/ARM/debuginfo-split-carryexpr.ll

18

It's strange that the DBG_VALUE MIs here are in reverse order (compared to IR). Perhaps that's worth filing a follow-up bug about?

23

Mind adding coverage for shr/shl?

[aprantl]

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

5561

This is a good change.

llvm/lib/IR/DebugInfoMetadata.cpp

1153 ↗

(On Diff #229310)

This is good too, but should land in a separate, preparatory patch, tested by a unit test in IR/MetadataTest.cpp.

[djtodoro]

llvm/test/CodeGen/ARM/debuginfo-split-carryexpr.ll

33

It is enough only producer: "clang version 10.0.0".

39

Same as above. (It is enough only producer: "clang version 10.0.0".)

41

test.cpp, /tmp

[Orlando]

llvm/test/CodeGen/ARM/debuginfo-split-carryexpr.ll

18

I reckon this is LiveDebugVariables. It should preserve order of DBG_VALUES for (all fragments of ) a particular source variable but not between variables. However, at the moment it doesn't correctly group the DBG_VALUES - DIExpression is used as part of the identity (so a different complex expression referring to the same or overlapping variable fragments is considered a different source variable) . My patch [0] fixes the grouping so only the inconsequential reordering of unrelated DBG_VALUES will happen.

[0] https://reviews.llvm.org/D70121

[StephenTozer]

Rebased off of merged parent, trimmed unnecessary/confusing test metadata.

[djtodoro]

LGTM. Thanks for doing this!

Please wait for someone from debug-info project to approve it as well.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

bool SelectionDAGBuilder::EmitFuncArgumentDbgValue(
   if (!Op) {
     // Create a DBG_VALUE for each decomposed value in ArgRegs to cover Reg
     auto splitMultiRegDbgValue
       = [&](ArrayRef<std::pair<unsigned, unsigned>> SplitRegs) {
       unsigned Offset = 0;
       for (auto RegAndSize : SplitRegs) {
         auto FragmentExpr = DIExpression::createFragmentExpression(
           Expr, Offset, RegAndSize.second);
-        if (!FragmentExpr)
+        // If a valid fragment expression cannot be created, the variable's
+        // correct value cannot be determined and so it is set as Undef.
+        if (!FragmentExpr) {
+          SDDbgValue *SDV = DAG.getConstantDbgValue(
+              Variable, Expr, UndefValue::get(V->getType()), DL, SDNodeOrder);
+          DAG.AddDbgValue(SDV, nullptr, false);
           continue;
+        }
         assert(!IsDbgDeclare && "DbgDeclare operand is not in memory?");
         FuncInfo.ArgDbgValues.push_back(
           BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), false,
                   RegAndSize.first, Variable, *FragmentExpr));
         Offset += RegAndSize.second;
       }
     };
 

llvm/test/CodeGen/ARM/debuginfo-split-carryexpr.ll

+; RUN: llc < %s -mtriple=armv7-linux-gnueabihf -stop-before=finalize-isel | FileCheck %s
+
+; When splitting-up integers during CodeGen, if the debug info contains
+; an expression with carry operators (i.e. arithmetic and shift ops), the
+; debug information will be dropped as these operators cannot be correctly
+; expressed across different registers.
+
+; CHECK: [[HIGH:![0-9]+]] = !DILocalVariable(name: "high"
+; CHECK: [[LOW:![0-9]+]] = !DILocalVariable(name: "low"
+;
+; As the debug information for "high" contains an arithmetic shift, while for
+; "low" it does not, only the former should be undefined.
+; CHECK-LABEL: body:
+; CHECK: [[LOWR:%[0-9]+]]:gpr = COPY $r0
+; CHECK: DBG_VALUE [[LOWR]], $noreg, [[LOW]]
+; CHECK: DBG_VALUE $noreg, $noreg, [[HIGH]]
+; CHECK: DBG_VALUE $noreg, $noreg, [[HIGH]]
+
+define dso_local i64 @_Z2fnx(i64 returned %value) local_unnamed_addr !dbg !7 {
+entry:
+  call void @llvm.dbg.value(metadata i64 %value, metadata !13, metadata !DIExpression()), !dbg !17
+  call void @llvm.dbg.value(metadata i64 %value, metadata !14, metadata !DIExpression(DW_OP_constu, 32, DW_OP_shra, DW_OP_LLVM_convert, 64, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value)), !dbg !17
+  call void @llvm.dbg.value(metadata i64 %value, metadata !16, metadata !DIExpression(DW_OP_LLVM_convert, 64, DW_ATE_unsigned, DW_OP_LLVM_convert, 32, DW_ATE_unsigned, DW_OP_stack_value)), !dbg !17
+  ret i64 %value, !dbg !18
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 10.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "test.cpp", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 10.0.0"}
+!7 = distinct !DISubprogram(name: "fn", linkageName: "_Z2fnx", scope: !8, file: !8, line: 2, type: !9, scopeLine: 2, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !12)
+!8 = !DIFile(filename: "test.cpp", directory: "/tmp")
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11, !11}
+!11 = !DIBasicType(name: "long long int", size: 64, encoding: DW_ATE_signed)
+!12 = !{!13, !14, !16}
+!13 = !DILocalVariable(name: "value", arg: 1, scope: !7, file: !8, line: 2, type: !11)
+!14 = !DILocalVariable(name: "high", scope: !7, file: !8, line: 3, type: !15)
+!15 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!16 = !DILocalVariable(name: "low", scope: !7, file: !8, line: 4, type: !15)
+!17 = !DILocation(line: 0, scope: !7)
+!18 = !DILocation(line: 5, column: 3, scope: !7)