[llvm] r237237 - [DebugInfo] Debug locations for constant SD nodes

Sergey Dmitrouk sdmitrouk at accesssoftek.com
Wed Jun 3 06:18:34 PDT 2015


On Tue, Jun 02, 2015 at 03:48:23PM -0700, David Blaikie wrote:
> > Not sure how to handle this yet, a couple of obvious tries causes test
> > failures, will look for something better.
>
> Given the debug info size/quality regression here - perhaps we could
> disable this again, temporarily, until we have an answer?

Looks like we don't need to do this.  I was wrong thinking that the fix
causes tests to fail, there were debugging aid kind of changes left in
the code, which I forgot to revert before running tests.

Please take a look at the attached patch.

Thanks,
Sergey
-------------- next part --------------
>From 08a8750c525427d92d77ec62d14e12b1de502479 Mon Sep 17 00:00:00 2001
From: Sergey Dmitrouk <sdmitrouk at accesssoftek.com>
Date: Wed, 3 Jun 2015 15:53:26 +0300
Subject: [PATCH] Erase constant dbgloc on reuse in PHI node

Basic block selection involves checking successor BBs for PHI nodes
that depend on the current BB.  In case such BBs are found, the value
being selected is a constant and such constant already exists in
current BB, it's value is reused.

This might lead to wrong locations in some situations, especially if
same constant value ends up being materialized twice in two different
ways, which discards that sharing and leaves us with wrong debug
location in the successor BB.

In code this involves the following sequence of calls:

 SelectionDAGBuilder::HandlePHINodesInSuccessorBlocks ->
 SelectionDAGBuilder::CopyValueToVirtualRegister ->
 SelectionDAGBuilder::getNonRegisterValue
---
 lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 12 +++-
 test/DebugInfo/X86/inlined-indirect-value.ll     | 79 ++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 test/DebugInfo/X86/inlined-indirect-value.ll

diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 9a81ec5..8a760bc 100644
--- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -1002,7 +1002,17 @@ bool SelectionDAGBuilder::findValue(const Value *V) const {
 SDValue SelectionDAGBuilder::getNonRegisterValue(const Value *V) {
   // If we already have an SDValue for this value, use it.
   SDValue &N = NodeMap[V];
-  if (N.getNode()) return N;
+  if (N.getNode()) {
+    if ((isa<ConstantSDNode>(N) || isa<ConstantFPSDNode>(N)) &&
+        N->getDebugLoc()) {
+      // Erase debug location from the node as it's about to be used at new
+      // place which we know nothing about to do not propagate wrong location.
+      // This is relevant to Constant and ConstantFP nodes because they can be
+      // used as constant expressions inside PHI nodes.
+      N->setDebugLoc(DebugLoc());
+    }
+    return N;
+  }
 
   // Otherwise create a new SDValue and remember it.
   SDValue Val = getValueImpl(V);
diff --git a/test/DebugInfo/X86/inlined-indirect-value.ll b/test/DebugInfo/X86/inlined-indirect-value.ll
new file mode 100644
index 0000000..ad470b6
--- /dev/null
+++ b/test/DebugInfo/X86/inlined-indirect-value.ll
@@ -0,0 +1,79 @@
+; RUN: llc -filetype=asm -asm-verbose=0 < %s | FileCheck %s
+
+; "1" from line 09 in the snippet below shouldn't be marked with location of "1"
+; from line 04.
+
+; options: -g -O3
+; 01 volatile int x;
+; 02 int y;
+; 03 static __attribute__((always_inline)) int f1() {
+; 04     if (x * 3 < 14) return 1;
+; 05     return 2;
+; 06 }
+; 07 int main() {
+; 08     x = f1();
+; 09     x = x ? 1 : 2;
+; 10 }
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at x = common global i32 0, align 4
+ at y = common global i32 0, align 4
+
+define i32 @main() {
+; CHECK: movl
+; CHECK-NEXT: movl $1
+
+entry:
+  %0 = load volatile i32, i32* @x, align 4, !dbg !16, !tbaa !19
+  %mul.i = mul nsw i32 %0, 3, !dbg !23
+  %cmp.i = icmp slt i32 %mul.i, 14, !dbg !24
+  %..i = select i1 %cmp.i, i32 1, i32 2, !dbg !25
+  store volatile i32 %..i, i32* @x, align 4, !dbg !27, !tbaa !19
+  %1 = load volatile i32, i32* @x, align 4, !dbg !28, !tbaa !19
+  %tobool = icmp ne i32 %1, 0, !dbg !28
+  br i1 %tobool, label %select.end, label %select.mid
+
+select.mid:                                       ; preds = %entry
+  br label %select.end
+
+select.end:                                       ; preds = %entry, %select.mid
+  %cond = phi i32 [ 1, %entry ], [ 2, %select.mid ]
+  store volatile i32 %cond, i32* @x, align 4, !dbg !29, !tbaa !19
+  ret i32 0, !dbg !30
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!13, !14}
+
+!0 = !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: 1, enums: !2, retainedTypes: !2, subprograms: !3, globals: !9, imports: !2)
+!1 = !DIFile(filename: "inline-break.c", directory: "/build/dir")
+!2 = !{}
+!3 = !{!4, !8}
+!4 = !DISubprogram(name: "main", scope: !1, file: !1, line: 7, type: !5, isLocal: false, isDefinition: true, scopeLine: 7, isOptimized: true, function: i32 ()* @main, variables: !2)
+!5 = !DISubroutineType(types: !6)
+!6 = !{!7}
+!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!8 = !DISubprogram(name: "f1", scope: !1, file: !1, line: 3, type: !5, isLocal: true, isDefinition: true, scopeLine: 3, isOptimized: true, variables: !2)
+!9 = !{!10, !12}
+!10 = !DIGlobalVariable(name: "x", scope: !0, file: !1, line: 1, type: !11, isLocal: false, isDefinition: true, variable: i32* @x)
+!11 = !DIDerivedType(tag: DW_TAG_volatile_type, baseType: !7)
+!12 = !DIGlobalVariable(name: "y", scope: !0, file: !1, line: 2, type: !7, isLocal: false, isDefinition: true, variable: i32* @y)
+!13 = !{i32 2, !"Dwarf Version", i32 4}
+!14 = !{i32 2, !"Debug Info Version", i32 3}
+!16 = !DILocation(line: 4, column: 9, scope: !17, inlinedAt: !18)
+!17 = distinct !DILexicalBlock(scope: !8, file: !1, line: 4, column: 9)
+!18 = distinct !DILocation(line: 8, column: 9, scope: !4)
+!19 = !{!20, !20, i64 0}
+!20 = !{!"int", !21, i64 0}
+!21 = !{!"omnipotent char", !22, i64 0}
+!22 = !{!"Simple C/C++ TBAA"}
+!23 = !DILocation(line: 4, column: 11, scope: !17, inlinedAt: !18)
+!24 = !DILocation(line: 4, column: 15, scope: !17, inlinedAt: !18)
+!25 = !DILocation(line: 4, column: 21, scope: !26, inlinedAt: !18)
+!26 = !DILexicalBlockFile(scope: !17, file: !1, discriminator: 1)
+!27 = !DILocation(line: 8, column: 7, scope: !4)
+!28 = !DILocation(line: 9, column: 9, scope: !4)
+!29 = !DILocation(line: 9, column: 7, scope: !4)
+!30 = !DILocation(line: 10, column: 1, scope: !4)
-- 
2.3.5



More information about the llvm-commits mailing list