EmitFuncArgumentDbgValue: Prefer stack slots over registers for stack arguments

[aprantl]

Summary

While investigating LLVM PR22316
http://llvm.org/bugs/show_bug.cgi?id=22316

I started wondering if it were not always preferable to emit the initial DBG_VALUEs for stack arguments as FI locations instead of describing the first register they get copied into. The advantage of doing this is that the arguments will be available as soon as the stack is setup. As illustrated by the testcase in the PR, the first copy of the FI into a register may be sunk by MachineSink.cpp into a later basic block. By describing the argument on the stack, we nicely circumvent this problem.

The COFF testcase change is the only testcase that broke with this change. Apparently it causes the order of the instructions in this function to get reshuffled without any functional change.

[aprantl]

Add context to diff.

[rnk]

test/DebugInfo/COFF/fpo-shrink-wrap.ll

16

This breaks the test case, though, now it isn't shrink-wrapped. That indicates that DBG_VALUE instructions change the codegen of shrink-wrapping. =(

We probably just need to skip DBG_VALUE instructions when doing the shrink-wrapping analysis.

[aprantl]

test/DebugInfo/COFF/fpo-shrink-wrap.ll

16

Oh! Good catch. That's I bug I should fix regardless!

[aprantl]

Rebase on top of https://reviews.llvm.org/D41187.

[rnk]

lgtm

lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

if (Reg && TargetRegisterInfo::isVirtualRegister(Reg)) {
         Reg = PR;
     }
     if (Reg) {
       Op = MachineOperand::CreateReg(Reg, false);
       IsIndirect = IsDbgDeclare;
     }
   }
 
+  if (!Op && N.getNode())
+    // Check if frame index is available.
+    if (LoadSDNode *LNode = dyn_cast<LoadSDNode>(N.getNode()))
+      if (FrameIndexSDNode *FINode =
+          dyn_cast<FrameIndexSDNode>(LNode->getBasePtr().getNode()))
+        Op = MachineOperand::CreateFI(FINode->getIndex());
+
   if (!Op) {
     // Check if ValueMap has reg number.
     DenseMap<const Value *, unsigned>::iterator VMI = FuncInfo.ValueMap.find(V);
     if (VMI != FuncInfo.ValueMap.end()) {
       const auto &TLI = DAG.getTargetLoweringInfo();
       RegsForValue RFV(V->getContext(), TLI, DAG.getDataLayout(), VMI->second,
                        V->getType(), isABIRegCopy(V));
       unsigned NumRegs =
if (VMI != FuncInfo.ValueMap.end()) {
         }
         return true;
       }
       Op = MachineOperand::CreateReg(VMI->second, false);
       IsIndirect = IsDbgDeclare;
     }
   }
 
-  if (!Op && N.getNode())
-    // Check if frame index is available.
-    if (LoadSDNode *LNode = dyn_cast<LoadSDNode>(N.getNode()))
-      if (FrameIndexSDNode *FINode =
-          dyn_cast<FrameIndexSDNode>(LNode->getBasePtr().getNode()))
-        Op = MachineOperand::CreateFI(FINode->getIndex());
-
   if (!Op)
     return false;
 
   assert(Variable->isValidLocationForIntrinsic(DL) &&
          "Expected inlined-at fields to agree");
   if (Op->isReg())
     FuncInfo.ArgDbgValues.push_back(
         BuildMI(MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect,

test/DebugInfo/COFF/fpo-shrink-wrap.ll

 ; RUN: llc -enable-shrink-wrap=true < %s | FileCheck %s --check-prefix=ASM
 ; RUN: llc -enable-shrink-wrap=true -filetype=obj < %s | llvm-readobj -codeview | FileCheck %s --check-prefix=OBJ
 
 ; C source:
 ; int doSomething(int*);
 ; int __fastcall shrink_wrap_basic(int a, int b, int c, int d) {
 ;   if (a < b)
 ;     return a;
 ;   for (int i = c; i < d; ++i)
 ;     doSomething(&c);
 ;   return doSomething(&c);
 ; }
 
 ; ASM: @shrink_wrap_basic@16:                  # @"\01@shrink_wrap_basic@16"
 ; ASM:         .cv_fpo_proc    @shrink_wrap_basic@16 8
-; ASM:         .cv_loc 0 1 3 9                 # t.c:3:9
-; ASM:         movl    %ecx, %eax
-; ASM:         cmpl    %edx, %eax
-; ASM:         jl      [[EPILOGUE:LBB0_[0-9]+]]
 
 ; ASM:         pushl   %ebx
 ; ASM:         .cv_fpo_pushreg %ebx
 ; ASM:         pushl   %edi
 ; ASM:         .cv_fpo_pushreg %edi
 ; ASM:         pushl   %esi
 ; ASM:         .cv_fpo_pushreg %esi
 ; ASM:         .cv_fpo_endprologue
 
+; ASM:         movl    %ecx, %eax
+; ASM:         .cv_loc 0 1 3 9                 # t.c:3:9
+; ASM:         cmpl    %edx, %eax
+; ASM:         jl      [[EPILOGUE:LBB0_[0-9]+]]
+
 ; ASM:         calll   _doSomething
 
+; ASM: [[EPILOGUE]]:                                 # %return
 ; ASM:         popl    %esi
 ; ASM:         popl    %edi
 ; ASM:         popl    %ebx
-; ASM: [[EPILOGUE]]:                                 # %return
 ; ASM:         retl    $8
-; ASM: Ltmp11:
+; ASM: Ltmp10:
 ; ASM:         .cv_fpo_endproc
 
 ; Note how RvaStart advances 7 bytes to skip the shrink-wrapped portion.
 ; OBJ: SubSectionType: FrameData (0xF5)
 ; OBJ:    FrameData {
 ; OBJ:      RvaStart: 0x0
 ; OBJ:      CodeSize: 0x34
 ; OBJ:      FrameFunc: $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + =
-; OBJ:      PrologSize: 0x9
+; OBJ:      PrologSize: 0x3
 ; OBJ:    }
 ; OBJ:    FrameData {
-; OBJ:      RvaStart: 0x7
-; OBJ:      CodeSize: 0x2D
+; OBJ:      RvaStart: 0x1
+; OBJ:      CodeSize: 0x33
 ; OBJ:      FrameFunc: $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + = $ebx $T0 4 - ^ =
 ; OBJ:      PrologSize: 0x2
 ; OBJ:    }
 ; OBJ:    FrameData {
-; OBJ:      RvaStart: 0x8
-; OBJ:      CodeSize: 0x2C
+; OBJ:      RvaStart: 0x2
+; OBJ:      CodeSize: 0x32
 ; OBJ:      FrameFunc: $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + = $ebx $T0 4 - ^ = $edi $T0 8 - ^ =
 ; OBJ:      PrologSize: 0x1
 ; OBJ:    }
 ; OBJ:    FrameData {
-; OBJ:      RvaStart: 0x9
-; OBJ:      CodeSize: 0x2B
+; OBJ:      RvaStart: 0x3
+; OBJ:      CodeSize: 0x31
 ; OBJ:      FrameFunc: $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + = $ebx $T0 4 - ^ = $edi $T0 8 - ^ = $esi $T0 12 - ^ =
 ; OBJ:      PrologSize: 0x0
 ; OBJ:    }
 ; OBJ-NOT: FrameData
 
 ; ModuleID = 't.c'
 source_filename = "t.c"
 target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"

test/DebugInfo/X86/stack-args.ll

+; RUN: llc -filetype=obj -o - %s | llvm-dwarfdump - | FileCheck %s
+; Generated from:
+; void * __attribute__ (( regparm(2) )) f(void *, void *);
+; void * __attribute__ (( regparm(0) )) g(void *, void *);
+;  
+; void *g(void *t, void *k) {
+;   if (k == (void *)0)
+;     return (void *)0;
+;   return f(t, k);
+; }
+
+; CHECK: DW_TAG_formal_parameter
+; CHECK-NEXT: DW_AT_location  (DW_OP_fbreg +4)
+; CHECK-NEXT: DW_AT_name  ("t")
+; CHECK: DW_TAG_formal_parameter
+; CHECK-NEXT: DW_AT_location  (DW_OP_fbreg +8)
+; CHECK-NEXT: DW_AT_name  ("k")
+
+source_filename = "t.c"
+target datalayout = "e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128"
+target triple = "i386--linux-gnu"
+
+; Function Attrs: nounwind
+define i8* @g(i8* %t, i8* %k) local_unnamed_addr #0 !dbg !10 {
+entry:
+  call void @llvm.dbg.value(metadata i8* %t, metadata !14, metadata !DIExpression()), !dbg !16
+  call void @llvm.dbg.value(metadata i8* %k, metadata !15, metadata !DIExpression()), !dbg !17
+  %cmp = icmp eq i8* %k, null, !dbg !18
+  br i1 %cmp, label %return, label %if.end, !dbg !20
+
+if.end:                                           ; preds = %entry
+  %call = tail call i8* @f(i8* inreg %t, i8* inreg nonnull %k) #3, !dbg !21
+  br label %return, !dbg !22
+
+return:                                           ; preds = %entry, %if.end
+  %retval.0 = phi i8* [ %call, %if.end ], [ null, %entry ]
+  ret i8* %retval.0, !dbg !23
+}
+
+declare i8* @f(i8* inreg, i8* inreg) local_unnamed_addr
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+attributes #0 = { nounwind  }
+attributes #2 = { nounwind readnone speculatable }
+attributes #3 = { nounwind }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!5, !6, !7, !8}
+!llvm.ident = !{!9}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 6.0.0  (llvm/trunk 319230)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3)
+!1 = !DIFile(filename: "t.c", directory: "/")
+!2 = !{}
+!3 = !{!4}
+!4 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: null, size: 32)
+!5 = !{i32 1, !"NumRegisterParameters", i32 0}
+!6 = !{i32 2, !"Dwarf Version", i32 4}
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!8 = !{i32 1, !"wchar_size", i32 4}
+!9 = !{!"clang version 6.0.0  (llvm/trunk 319230)"}
+!10 = distinct !DISubprogram(name: "g", scope: !1, file: !1, line: 4, type: !11, isLocal: false, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !13)
+!11 = !DISubroutineType(types: !12)
+!12 = !{!4, !4, !4}
+!13 = !{!14, !15}
+!14 = !DILocalVariable(name: "t", arg: 1, scope: !10, file: !1, line: 4, type: !4)
+!15 = !DILocalVariable(name: "k", arg: 2, scope: !10, file: !1, line: 4, type: !4)
+!16 = !DILocation(line: 4, column: 15, scope: !10)
+!17 = !DILocation(line: 4, column: 24, scope: !10)
+!18 = !DILocation(line: 5, column: 9, scope: !19)
+!19 = distinct !DILexicalBlock(scope: !10, file: !1, line: 5, column: 7)
+!20 = !DILocation(line: 5, column: 7, scope: !10)
+!21 = !DILocation(line: 7, column: 10, scope: !10)
+!22 = !DILocation(line: 7, column: 3, scope: !10)
+!23 = !DILocation(line: 8, column: 1, scope: !10)