[PATCH] D70248: Disallow shift operations in debug expressions spanning multiple registers

Thu Nov 14 09:02:13 PST 2019

StephenTozer created this revision.
StephenTozer added a project: LLVM.
Herald added subscribers: llvm-commits, ormris, hiraditya, aprantl.
StephenTozer added reviewers: markus, vsk, probinson, aprantl, dblaikie, djtodoro.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D70248

Files:
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/DebugInfoMetadata.cpp
  llvm/test/CodeGen/ARM/debuginfo-split-carryexpr.ll


Index: llvm/test/CodeGen/ARM/debuginfo-split-carryexpr.ll
===================================================================

--- /dev/null
+++ llvm/test/CodeGen/ARM/debuginfo-split-carryexpr.ll
@@ -0,0 +1,50 @@
+; 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 !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 (https://github.com/llvm/llvm-project.git 2f8a9f0eb221bb85164bbd81179f3a15630d24e2)", 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 (https://github.com/llvm/llvm-project.git 2f8a9f0eb221bb85164bbd81179f3a15630d24e2)"}
+!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: "./most.cpp", directory: "/home/stozer/dev/llvm-project")
+!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)
Index: llvm/lib/IR/DebugInfoMetadata.cpp
===================================================================
--- llvm/lib/IR/DebugInfoMetadata.cpp
+++ llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1148,10 +1148,13 @@
     for (auto Op : Expr->expr_ops()) {
       switch (Op.getOp()) {
       default: break;
+      case dwarf::DW_OP_shr:
+      case dwarf::DW_OP_shra:
+      case dwarf::DW_OP_shl:
       case dwarf::DW_OP_plus:
       case dwarf::DW_OP_minus:
-        // We can't safely split arithmetic into multiple fragments because we
-        // can't express carry-over between fragments.
+        // We can't safely split arithmetic or shift operations into multiple
+        // fragments because we can't express carry-over between fragments.
         //
         // FIXME: We *could* preserve the lowest fragment of a constant offset
         // operation if the offset fits into SizeInBits.
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===================================================================
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -5534,8 +5534,14 @@
       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,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70248.229310.patch
Type: text/x-patch
Size: 5105 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191114/9e35f5b0/attachment-0001.bin>
