[SelectionDAG] Remove debug locations from ConstantSD(FP)Nodes

[vsk]

Summary

This removes debug locations from ConstantSDNode and ConstantSDFPNode.

When this kind of node is materialized we no longer create a line table
entry which jumps back to the constant's first point of use. This makes
single-stepping behavior smoother, and it matches the model used by IR,
where Constants have no locations. See this thread for more context:

http://lists.llvm.org/pipermail/llvm-dev/2018-June/124164.html

I'd like to handle constant BuildVectorSDNodes and to try to eliminate
passing SDLocs to SelectionDAG::getConstant*() in follow-up commits.

[echristo]

LGTM.

[yroux]

Hi Vedant,

This patch broke ARM bots since expected line info is missing in
address sanitizer output for null_redef.cc testcase, notice that when
the test is compiled in thumb mode the line info is still present.

logs available at:

http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/1203/steps/ninja%20check%201/logs/FAIL%3A%20AddressSanitizer-armhf-linux%3A%3Anull_deref.cc

Thanks
Yvan

[petpav01]

Hi Vedant,

This change also made debug information for the following example slightly worse:

$ cat test.c
void bar(void);
int foo(void) {
  bar();
  int i = 2;
  return i;
}
$ clang -target armv7a-none-eabi -g -c test.c main.c
$ llvm-objdump -S test.o

test.o: file format ELF32-arm-little

Disassembly of section .text:
foo:
; int foo(void) {
       0:       00 48 2d e9     push    {r11, lr}
       4:       0d b0 a0 e1     mov     r11, sp
       8:       08 d0 4d e2     sub     sp, sp, #8
; bar();
       c:       fe ff ff eb     bl      #-8 <foo+0xc>
      10:       02 e0 a0 e3     mov     lr, #2
; int i = 2;
      14:       04 e0 8d e5     str     lr, [sp, #4]
; return i;
      18:       04 00 9d e5     ldr     r0, [sp, #4]
      1c:       0b d0 a0 e1     mov     sp, r11
      20:       00 88 bd e8     pop     {r11, pc}

Instruction mov lr, #2 is recorded as part of line 3 (bar();) but previously it was more accurately accounted as line 4 (int i = 2;).

Previous behaviour:

$ llvm-objdump -S test.o

test.o: file format ELF32-arm-little

Disassembly of section .text:
foo:
; int foo(void) {
       0:       00 48 2d e9     push    {r11, lr}
       4:       0d b0 a0 e1     mov     r11, sp
       8:       08 d0 4d e2     sub     sp, sp, #8
; bar();
       c:       fe ff ff eb     bl      #-8 <foo+0xc>
; int i = 2;
      10:       02 e0 a0 e3     mov     lr, #2
      14:       04 e0 8d e5     str     lr, [sp, #4]
; return i;
      18:       04 00 9d e5     ldr     r0, [sp, #4]
      1c:       0b d0 a0 e1     mov     sp, r11
      20:       00 88 bd e8     pop     {r11, pc}

It is not clear to me whether this is an expected change but I thought I would let you know about it if it actually isn't.

Thanks,
Petr

[vsk]

From @yroux:

Hi Vedant,

This patch broke ARM bots since expected line info is missing in
address sanitizer output for null_redef.cc testcase, notice that when
the test is compiled in thumb mode the line info is still present.

logs available at:

http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/1203/steps/ninja%20check%201/logs/FAIL%3A%20AddressSanitizer-armhf-linux%3A%3Anull_deref.cc

Thanks
Yvan

Phab didn't pick up my reply from the mailing list, so just to recap: thanks for bringing this to my attention -- I've relaxed the test and the bot has recovered.

In D48468#1147627, @petpav01 wrote:

Hi Vedant,

This change also made debug information for the following example slightly worse:

$ cat test.c
void bar(void);
int foo(void) {
  bar();
  int i = 2;
  return i;
}
$ clang -target armv7a-none-eabi -g -c test.c main.c
$ llvm-objdump -S test.o

test.o: file format ELF32-arm-little

Disassembly of section .text:
foo:
; int foo(void) {
       0:       00 48 2d e9     push    {r11, lr}
       4:       0d b0 a0 e1     mov     r11, sp
       8:       08 d0 4d e2     sub     sp, sp, #8
; bar();
       c:       fe ff ff eb     bl      #-8 <foo+0xc>
      10:       02 e0 a0 e3     mov     lr, #2
; int i = 2;
      14:       04 e0 8d e5     str     lr, [sp, #4]
; return i;
      18:       04 00 9d e5     ldr     r0, [sp, #4]
      1c:       0b d0 a0 e1     mov     sp, r11
      20:       00 88 bd e8     pop     {r11, pc}

Instruction mov lr, #2 is recorded as part of line 3 (bar();) but previously it was more accurately accounted as line 4 (int i = 2;).

Previous behaviour:

$ llvm-objdump -S test.o

test.o: file format ELF32-arm-little

Disassembly of section .text:
foo:
; int foo(void) {
       0:       00 48 2d e9     push    {r11, lr}
       4:       0d b0 a0 e1     mov     r11, sp
       8:       08 d0 4d e2     sub     sp, sp, #8
; bar();
       c:       fe ff ff eb     bl      #-8 <foo+0xc>
; int i = 2;
      10:       02 e0 a0 e3     mov     lr, #2
      14:       04 e0 8d e5     str     lr, [sp, #4]
; return i;
      18:       04 00 9d e5     ldr     r0, [sp, #4]
      1c:       0b d0 a0 e1     mov     sp, r11
      20:       00 88 bd e8     pop     {r11, pc}

It is not clear to me whether this is an expected change but I thought I would let you know about it if it actually isn't.

Thanks,
Petr

Hi @petpav01, the behavior difference you're seeing is an expected result of this patch. The tradeoff is to give up some debug info precision on single-use constant materialization in order to improve single-stepping when constants are reused. I'm happy to try and find a better solution if this regresses some behavior you depend on.

[aprantl]

In other words, the mov lr, #2 is the *materialization* of the constant and is no longer associated with a specific source location.

[yroux]

Thanks for the fix Vedant.

[petpav01]

Thanks for the explanation. This does not cause any immediate problem for us but it is unfortunate that a simple assignment of a constant now appears to be split between two lines. Hopefully it will be potentially possible to implement some improvement for this case in the future.

llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h

public:
   }
 };
 
 class ConstantSDNode : public SDNode {
   friend class SelectionDAG;
 
   const ConstantInt *Value;
 
-  ConstantSDNode(bool isTarget, bool isOpaque, const ConstantInt *val,
-                 const DebugLoc &DL, EVT VT)
-      : SDNode(isTarget ? ISD::TargetConstant : ISD::Constant, 0, DL,
+  ConstantSDNode(bool isTarget, bool isOpaque, const ConstantInt *val, EVT VT)
+      : SDNode(isTarget ? ISD::TargetConstant : ISD::Constant, 0, DebugLoc(),
                getSDVTList(VT)),
         Value(val) {
     ConstantSDNodeBits.IsOpaque = isOpaque;
   }
 
 public:
   const ConstantInt *getConstantIntValue() const { return Value; }
   const APInt &getAPIntValue() const { return Value->getValue(); }
uint64_t SDNode::getConstantOperandVal(unsigned Num) const {
   return cast<ConstantSDNode>(getOperand(Num))->getZExtValue();
 }
 
 class ConstantFPSDNode : public SDNode {
   friend class SelectionDAG;
 
   const ConstantFP *Value;
 
-  ConstantFPSDNode(bool isTarget, const ConstantFP *val, const DebugLoc &DL,
-                   EVT VT)
-      : SDNode(isTarget ? ISD::TargetConstantFP : ISD::ConstantFP, 0, DL,
-               getSDVTList(VT)),
+  ConstantFPSDNode(bool isTarget, const ConstantFP *val, EVT VT)
+      : SDNode(isTarget ? ISD::TargetConstantFP : ISD::ConstantFP, 0,
+               DebugLoc(), getSDVTList(VT)),
         Value(val) {}
 
 public:
   const APFloat& getValueAPF() const { return Value->getValueAPF(); }
   const ConstantFP *getConstantFPValue() const { return Value; }
 
   /// Return true if the value is positive or negative zero.
   bool isZero() const { return Value->isZero(); }

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

SDValue SelectionDAG::getConstant(const ConstantInt &Val, const SDLoc &DL,
   ID.AddBoolean(isO);
   void *IP = nullptr;
   SDNode *N = nullptr;
   if ((N = FindNodeOrInsertPos(ID, DL, IP)))
     if (!VT.isVector())
       return SDValue(N, 0);
 
   if (!N) {
-    N = newSDNode<ConstantSDNode>(isT, isO, Elt, DL.getDebugLoc(), EltVT);
+    N = newSDNode<ConstantSDNode>(isT, isO, Elt, EltVT);
     CSEMap.InsertNode(N, IP);
     InsertNode(N);
     NewSDValueDbgMsg(SDValue(N, 0), "Creating constant: ", this);
   }
 
   SDValue Result(N, 0);
   if (VT.isVector())
     Result = getSplatBuildVector(VT, DL, Result);
SDValue SelectionDAG::getConstantFP(const ConstantFP &V, const SDLoc &DL,
   ID.AddPointer(&V);
   void *IP = nullptr;
   SDNode *N = nullptr;
   if ((N = FindNodeOrInsertPos(ID, DL, IP)))
     if (!VT.isVector())
       return SDValue(N, 0);
 
   if (!N) {
-    N = newSDNode<ConstantFPSDNode>(isTarget, &V, DL.getDebugLoc(), EltVT);
+    N = newSDNode<ConstantFPSDNode>(isTarget, &V, EltVT);
     CSEMap.InsertNode(N, IP);
     InsertNode(N);
   }
 
   SDValue Result(N, 0);
   if (VT.isVector())
     Result = getSplatBuildVector(VT, DL, Result);
   NewSDValueDbgMsg(Result, "Creating fp constant: ", this);

llvm/trunk/test/CodeGen/AArch64/machine-outliner-remarks.ll

 ; RUN: llc %s -enable-machine-outliner -mtriple=aarch64-unknown-unknown -pass-remarks=machine-outliner -pass-remarks-missed=machine-outliner -o /dev/null 2>&1 | FileCheck %s
-; CHECK: machine-outliner-remarks.ll:5:9:
+; CHECK: <unknown>:0:0:
 ; CHECK-SAME: Did not outline 2 instructions from 2 locations.
 ; CHECK-SAME: Bytes from outlining all occurrences (36) >=
 ; CHECK-SAME: Unoutlined instruction bytes (16)
-; CHECK-SAME: (Also found at: machine-outliner-remarks.ll:13:9)
+; CHECK-SAME: (Also found at: <UNKNOWN LOCATION>)
 ; CHECK: remark: <unknown>:0:0: Saved 20 bytes by outlining 12 instructions
-; CHECK-SAME: from 2 locations. (Found at: machine-outliner-remarks.ll:36:1,
-; CHECK-SAME: machine-outliner-remarks.ll:27:9)
+; CHECK-SAME: from 2 locations. (Found at: <UNKNOWN LOCATION>,
+; CHECK-SAME: <UNKNOWN LOCATION>)
 ; RUN: llc %s -enable-machine-outliner -mtriple=aarch64-unknown-unknown -o /dev/null -pass-remarks-missed=machine-outliner -pass-remarks-output=%t.yaml
 ; RUN: cat %t.yaml | FileCheck %s -check-prefix=YAML
 ; YAML: --- !Missed
 ; YAML-NEXT: Pass:            machine-outliner
 ; YAML-NEXT: Name:            NotOutliningCheaper
-; YAML-NEXT: DebugLoc:        { File: machine-outliner-remarks.ll, Line: 5, Column: 9 }
 ; YAML-NEXT: Function:        dog
 ; YAML-NEXT: Args:            
 ; YAML-NEXT:   - String:          'Did not outline '
 ; YAML-NEXT:   - Length:          '2'
 ; YAML-NEXT:   - String:          ' instructions'
 ; YAML-NEXT:   - String:          ' from '
 ; YAML-NEXT:   - NumOccurrences:  '2'
 ; YAML-NEXT:   - String:          ' locations.'
 ; YAML-NEXT:   - String:          ' Bytes from outlining all occurrences ('
 ; YAML-NEXT:   - OutliningCost:   '36'
 ; YAML-NEXT:   - String:          ')'
 ; YAML-NEXT:   - String:          ' >= Unoutlined instruction bytes ('
 ; YAML-NEXT:   - NotOutliningCost: '16'
 ; YAML-NEXT:   - String:          ')'
 ; YAML-NEXT:   - String:          ' (Also found at: '
-; YAML-NEXT:   - OtherStartLoc1:  'machine-outliner-remarks.ll:13:9'
-; YAML-NEXT:     DebugLoc:        { File: machine-outliner-remarks.ll, Line: 13, Column: 9 }
+; YAML-NEXT:   - OtherStartLoc1:  '<UNKNOWN LOCATION>'
 ; YAML-NEXT:   - String:          ')'
 ; YAML: --- !Passed
 ; YAML-NEXT: Pass:            machine-outliner
 ; YAML-NEXT: Name:            OutlinedFunction
 ; YAML-NEXT: Function:        OUTLINED_FUNCTION_0
 ; YAML-NEXT: Args:            
 ; YAML-NEXT:   - String:          'Saved '
 ; YAML-NEXT:   - OutliningBenefit: '20'
 ; YAML-NEXT:   - String:          ' bytes by '
 ; YAML-NEXT:   - String:          'outlining '
 ; YAML-NEXT:   - Length:          '12'
 ; YAML-NEXT:   - String:          ' instructions '
 ; YAML-NEXT:   - String:          'from '
 ; YAML-NEXT:   - NumOccurrences:  '2'
 ; YAML-NEXT:   - String:          ' locations. '
 ; YAML-NEXT:   - String:          '(Found at: '
-; YAML-NEXT:   - StartLoc0:       'machine-outliner-remarks.ll:36:1'
-; YAML-NEXT:     DebugLoc:        { File: machine-outliner-remarks.ll, Line: 36, Column: 1 }
+; YAML-NEXT:   - StartLoc0:       '<UNKNOWN LOCATION>'
 ; YAML-NEXT:   - String:          ', '
-; YAML-NEXT:   - StartLoc1:       'machine-outliner-remarks.ll:27:9'
-; YAML-NEXT:     DebugLoc:        { File: machine-outliner-remarks.ll, Line: 27, Column: 9 }
+; YAML-NEXT:   - StartLoc1:       '<UNKNOWN LOCATION>'
 ; YAML-NEXT:   - String:          ')'
 
 define void @dog() #0 !dbg !8 {
 entry:
   %x = alloca i32, align 4
   %y = alloca i32, align 4
   store i32 0, i32* %x, align 4
   store i32 1, i32* %y, align 4, !dbg !12

llvm/trunk/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll

 ; RUN: llc -filetype=asm -asm-verbose=0 < %s | FileCheck %s
 
 ; int main()
 ; {
 ;     int x = 0;
 ;     if (x > 0)
 ;         return x;
 ;     x = -1; // <== this line should have correct debug location
 ;     return -1;
 ; }
 
-; CHECK: .loc 1 6 7
-; CHECK: mvn
-
 target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
 target triple = "armv7--linux-gnueabihf"
 
 ; Function Attrs: nounwind
 define i32 @main() !dbg !4 {
 entry:
   %retval = alloca i32, align 4
   %x = alloca i32, align 4
   store i32 0, i32* %retval
   call void @llvm.dbg.declare(metadata i32* %x, metadata !10, metadata !11), !dbg !12
   store i32 0, i32* %x, align 4, !dbg !12
   %0 = load i32, i32* %x, align 4, !dbg !13
   %cmp = icmp sgt i32 %0, 0, !dbg !15
   br i1 %cmp, label %if.then, label %if.end, !dbg !16
 
 if.then:                                          ; preds = %entry
   %1 = load i32, i32* %x, align 4, !dbg !17
   store i32 %1, i32* %retval, !dbg !18
   br label %return, !dbg !18
 
 if.end:                                           ; preds = %entry
+; Materialize the constant.
+; CHECK:      .loc    1 0
+; CHECK-NEXT: mvn     r0, #0
+
+; The backend performs the store to %retval first, for some reason.
+; CHECK-NEXT: .loc    1 7 5
+; CHECK-NEXT: str     r0, [sp, #4]
   store i32 -1, i32* %x, align 4, !dbg !19
+
+; CHECK-NEXT: .loc    1 6 7
+; CHECK-NEXT: str     r0, [sp]
   store i32 -1, i32* %retval, !dbg !20
   br label %return, !dbg !20
 
 return:                                           ; preds = %if.end, %if.then
   %2 = load i32, i32* %retval, !dbg !21
   ret i32 %2, !dbg !21
 }
 

llvm/trunk/test/DebugInfo/Generic/constant-sdnodes-have-dbg-location.ll

-; RUN: llc -debug -dag-dump-verbose < %s 2>&1 | FileCheck %s
-; REQUIRES: asserts
-
-; CHECK: t{{[0-9]+}}: i32 = Constant<-1>test.c:4:5
-
-define i32 @main() !dbg !4 {
-entry:
-  %retval = alloca i32, align 4
-  store i32 0, i32* %retval
-  ret i32 -1, !dbg !10
-}
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!8, !9}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: false, emissionKind: FullDebug)
-!1 = !DIFile(filename: "test.c", directory: "/home/user/clang-llvm/build")
-!2 = !{}
-!4 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 2, type: !5, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
-!5 = !DISubroutineType(types: !6)
-!6 = !{!7}
-!7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
-!8 = !{i32 2, !"Dwarf Version", i32 4}
-!9 = !{i32 2, !"Debug Info Version", i32 3}
-!10 = !DILocation(line: 4, column: 5, scope: !4)

llvm/trunk/test/DebugInfo/Generic/constantfp-sdnodes-have-dbg-location.ll

-; RUN: llc -debug -dag-dump-verbose < %s 2>&1 | FileCheck %s
-; REQUIRES: asserts
-
-; CHECK: t{{[0-9]+}}: f64 = ConstantFP<1.500000e+00>test.c:3:5
-
-define double @f() !dbg !4 {
-entry:
-  ret double 1.500000e+00, !dbg !10
-}
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!8, !9}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: false, emissionKind: FullDebug)
-!1 = !DIFile(filename: "test.c", directory: "/home/user/clang-llvm/build")
-!2 = !{}
-!4 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 1, type: !5, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
-!5 = !DISubroutineType(types: !6)
-!6 = !{!7}
-!7 = !DIBasicType(name: "double", size: 64, align: 64, encoding: DW_ATE_float)
-!8 = !{i32 2, !"Dwarf Version", i32 4}
-!9 = !{i32 2, !"Debug Info Version", i32 3}
-!10 = !DILocation(line: 3, column: 5, scope: !4)

llvm/trunk/test/tools/llvm-objdump/AMDGPU/source-lines.ll

 ; RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %s > %t.ll
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa-amdgiz -mcpu=gfx802 -filetype=obj -O0 -o %t.o %t.ll
 ; RUN: llvm-objdump -triple=amdgcn-amd-amdhsa-amdgiz -mcpu=gfx802 -disassemble -line-numbers %t.o | FileCheck --check-prefix=LINE %t.ll
 ; RUN: llvm-objdump -triple=amdgcn-amd-amdhsa-amdgiz -mcpu=gfx802 -disassemble -source %t.o | FileCheck --check-prefix=SOURCE %t.ll
 
 ; Prologue.
 ; LINE:      source_lines_test:
 ; LINE-NEXT: ; {{.*}}source-lines.cl:1
 ; Kernel.
-; LINE: ; {{.*}}source-lines.cl:2
 ; LINE: v_mov_b32_e32 v{{[0-9]+}}, 0x777
-; LINE: ; {{.*}}source-lines.cl:3
+; LINE: ; {{.*}}source-lines.cl:2
 ; LINE: v_mov_b32_e32 v{{[0-9]+}}, 0x888
+; LINE: ; {{.*}}source-lines.cl:3
 ; LINE: ; {{.*}}source-lines.cl:4
 ; LINE: v_add_u32_e32
 ; LINE: ; {{.*}}source-lines.cl:5
 ; LINE: flat_store_dword
 ; Epilogue.
 ; LINE:      ; {{.*}}source-lines.cl:6
 ; LINE-NEXT: s_endpgm
 
 ; Prologue.
 ; SOURCE:      source_lines_test:
 ; SOURCE-NEXT: ; kernel void source_lines_test(global int *Out) {
 ; Kernel.
-; SOURCE: ; int var0 = 0x777;
 ; SOURCE: v_mov_b32_e32 v{{[0-9]+}}, 0x777
-; SOURCE: ; int var1 = 0x888;
+; SOURCE: ; int var0 = 0x777;
 ; SOURCE: v_mov_b32_e32 v{{[0-9]+}}, 0x888
+; SOURCE: ; int var1 = 0x888;
 ; SOURCE: ; int var2 = var0 + var1;
 ; SOURCE: v_add_u32_e32
 ; SOURCE: ; *Out = var2;
 ; SOURCE: flat_store_dword
 ; Epilogue.
 ; SOURCE:      ; }
 ; SOURCE-NEXT: s_endpgm