[mips] Fix the position of the .loc prologue_end directive in cases when the generated code contains DBG_VALUE instruction.

[dsanders]

Summary

[mips] Fix the position of the .loc prologue_end directive in cases when the generated code contains DBG_VALUE instruction.

[echristo]

This seems to be a bit of a cut and paste conditional. I haven't had much of a chance to look, but is there something else that can be done here?

-eric

[dsanders]

According to findPrologueEndLoc(), the source location from the first non-DBG_VALUE instruction without the FrameSetup flag and with a known source location gets the prologue_end marker. I've had a quick look through the other targets and there seems to be two ways that targets influence this. PowerPC and NVPTX simply uses an unknown location in their prologue (via DebugLoc()), while ARM and AArch64 annotate some (but not all) instructions with the MachineInstr::FrameSetup flag. X86 seems to do both. Other targets didn't seem to do anything about it.

I think we could safely use an unknown DebugLoc() in all cases since the line number shouldn't change until the end of the prologue. Does anyone know of any benefits to using the FrameSetup flag? It seems preferable to use it so that prologue instructions have the location of the start of the function but I doubt that it has a real effect on the output.

[sstankovic]

Does anyone know of any benefits to using the FrameSetup flag? It seems preferable to use it so that prologue instructions have the location of the start of the function but I doubt that it has a real effect on the output.

It seems the only part of LLVM that uses FrameSetup flag is the function calculateDbgValueHistory that calculates the history for local variables. I did quick check by changing Mips backend to add FrameSetup flag to some (but not all) of the instructions in the prologue, and the generated debug info in .s files was as before the change.

[sstankovic]

Rebased the patch. Created new method MipsInstrInfo::getDebugLoc().

This seems to be a bit of a cut and paste conditional. I haven't had much of a chance to look, but is there something else that can be done here?

I moved the logic that creates DebugLoc to a new method.

[echristo]

One small inline comment.

lib/Target/Mips/MipsSEISelDAGToDAG.cpp

133–134

The subtarget in MipsISelDAGToDAG is MipsSubtarget so you shouldn't need the cast.

[sstankovic]

Removed the unnecessary cast.

[sstankovic]

Is there anything else that needs to be done for this patch?

[dsanders]

Closing an old differential revision. It was resolved by another patch

lib/Target/Mips/MipsInstrInfo.h

virtual void loadRegFromStack(MachineBasicBlock &MBB,
                                 const TargetRegisterInfo *TRI,
                                 int64_t Offset) const = 0;
 
   /// Create an instruction which has the same operands and memory operands
   /// as MI but has a new opcode.
   MachineInstrBuilder genInstrWithNewOpc(unsigned NewOpc,
                                          MachineBasicBlock::iterator I) const;
 
+  DebugLoc
+  getDebugLoc(MachineBasicBlock::iterator I, MachineBasicBlock &MBB) const {
+    return (I != MBB.end() && !I->isDebugValue()) ? I->getDebugLoc()
+                                                  : DebugLoc();
+  }
+
 protected:
   bool isZeroImm(const MachineOperand &op) const;
 
   MachineMemOperand *GetMemOperand(MachineBasicBlock &MBB, int FI,
                                    unsigned Flag) const;
 
 private:
   virtual unsigned getAnalyzableBrOpc(unsigned Opc) const = 0;

lib/Target/Mips/MipsSEFrameLowering.cpp

void MipsSEFrameLowering::emitPrologue(MachineFunction &MF) const {
   MipsFunctionInfo *MipsFI = MF.getInfo<MipsFunctionInfo>();
 
   const MipsSEInstrInfo &TII =
       *static_cast<const MipsSEInstrInfo *>(STI.getInstrInfo());
   const MipsRegisterInfo &RegInfo =
       *static_cast<const MipsRegisterInfo *>(STI.getRegisterInfo());
 
   MachineBasicBlock::iterator MBBI = MBB.begin();
-  DebugLoc dl = MBBI != MBB.end() ? MBBI->getDebugLoc() : DebugLoc();
+  DebugLoc dl = TII.getDebugLoc(MBBI, MBB);
   unsigned SP = STI.isABI_N64() ? Mips::SP_64 : Mips::SP;
   unsigned FP = STI.isABI_N64() ? Mips::FP_64 : Mips::FP;
   unsigned ZERO = STI.isABI_N64() ? Mips::ZERO_64 : Mips::ZERO;
   unsigned ADDu = STI.isABI_N64() ? Mips::DADDu : Mips::ADDu;
 
   // First, compute final stack size.
   uint64_t StackSize = MFI->getStackSize();
 

lib/Target/Mips/MipsSEISelDAGToDAG.cpp

void MipsSEDAGToDAGISel::initGlobalBaseReg(MachineFunction &MF) {
   MipsFunctionInfo *MipsFI = MF.getInfo<MipsFunctionInfo>();
 
   if (!MipsFI->globalBaseRegSet())
     return;
 
   MachineBasicBlock &MBB = MF.front();
   MachineBasicBlock::iterator I = MBB.begin();
   MachineRegisterInfo &RegInfo = MF.getRegInfo();
-  const TargetInstrInfo &TII = *Subtarget->getInstrInfo();
-  DebugLoc DL = I != MBB.end() ? I->getDebugLoc() : DebugLoc();
+  const MipsInstrInfo &TII = *Subtarget->getInstrInfo();
+  DebugLoc DL = TII.getDebugLoc(I, MBB);
   unsigned V0, V1, GlobalBaseReg = MipsFI->getGlobalBaseReg();
   const TargetRegisterClass *RC;
   const MipsABIInfo &ABI = static_cast<const MipsTargetMachine &>(TM).getABI();
   RC = (ABI.IsN64()) ? &Mips::GPR64RegClass : &Mips::GPR32RegClass;
 
   V0 = RegInfo.createVirtualRegister(RC);
   V1 = RegInfo.createVirtualRegister(RC);
 

lib/Target/Mips/MipsSEInstrInfo.cpp

if (ZeroReg)
     MIB.addReg(ZeroReg);
 }
 
 void MipsSEInstrInfo::
 storeRegToStack(MachineBasicBlock &MBB, MachineBasicBlock::iterator I,
                 unsigned SrcReg, bool isKill, int FI,
                 const TargetRegisterClass *RC, const TargetRegisterInfo *TRI,
                 int64_t Offset) const {
-  DebugLoc DL;
-  if (I != MBB.end()) DL = I->getDebugLoc();
+  DebugLoc DL = getDebugLoc(I, MBB);
   MachineMemOperand *MMO = GetMemOperand(MBB, FI, MachineMemOperand::MOStore);
 
   unsigned Opc = 0;
 
   if (Mips::GPR32RegClass.hasSubClassEq(RC))
     Opc = Mips::SW;
   else if (Mips::GPR64RegClass.hasSubClassEq(RC))
     Opc = Mips::SD;
unsigned MipsSEInstrInfo::getOppositeBranchOpc(unsigned Opc) const {
   }
 }
 
 /// Adjust SP by Amount bytes.
 void MipsSEInstrInfo::adjustStackPtr(unsigned SP, int64_t Amount,
                                      MachineBasicBlock &MBB,
                                      MachineBasicBlock::iterator I) const {
   const MipsSubtarget &STI = Subtarget;
-  DebugLoc DL = I != MBB.end() ? I->getDebugLoc() : DebugLoc();
+  DebugLoc DL = getDebugLoc(I, MBB);
   unsigned ADDu = STI.isABI_N64() ? Mips::DADDu : Mips::ADDu;
   unsigned ADDiu = STI.isABI_N64() ? Mips::DADDiu : Mips::ADDiu;
 
   if (isInt<16>(Amount))// addi sp, sp, amount
     BuildMI(MBB, I, DL, get(ADDiu), SP).addReg(SP).addImm(Amount);
   else { // Expand immediate that doesn't fit in 16-bit.
     unsigned Reg = loadImmediate(Amount, MBB, I, DL, nullptr);
     BuildMI(MBB, I, DL, get(ADDu), SP).addReg(SP).addReg(Reg, RegState::Kill);

test/DebugInfo/Mips/delay-slot.ll

 ; RUN: llc -filetype=obj -O0 < %s -mtriple mips-unknown-linux-gnu | llvm-dwarfdump - | FileCheck %s
 ; PR19815
 
 ; Generated using clang -target mips-linux-gnu -g test.c -S -o - -flto|opt -sroa -S
 ; test.c:
 ;
 ; int foo(int x) {
 ;  if (x)
 ;    return 0;
 ;  return 1;
 ; }
 
 ; CHECK: Address            Line   Column File   ISA Discriminator Flags
 ; CHECK: ------------------ ------ ------ ------ --- ------------- -------------
 ; CHECK: 0x0000000000000000      1      0      1   0             0  is_stmt
-; CHECK: 0x0000000000000000      1      0      1   0             0  is_stmt prologue_end
-; CHECK: 0x0000000000000008      2      0      1   0             0  is_stmt
+; CHECK: 0x0000000000000008      2      0      1   0             0  is_stmt prologue_end
 ; CHECK: 0x0000000000000020      3      0      1   0             0  is_stmt
 ; CHECK: 0x0000000000000030      4      0      1   0             0  is_stmt
 ; CHECK: 0x0000000000000040      5      0      1   0             0  is_stmt
 ; CHECK: 0x0000000000000050      5      0      1   0             0  is_stmt end_sequence
 
 target datalayout = "E-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64"
 target triple = "mips--linux-gnu"
 

test/DebugInfo/Mips/prologue_end.ll

+; RUN: llc -mtriple=mips-linux-gnu -O0 -relocation-model=static < %s | FileCheck %s -check-prefix=STATIC
+; RUN: llc -mtriple=mips-linux-gnu -O0 -relocation-model=static -disable-fp-elim < %s | FileCheck %s -check-prefix=STATIC-FP
+; RUN: llc -mtriple=mips-linux-gnu -O0 -relocation-model=pic < %s | FileCheck %s -check-prefix=PIC
+; RUN: llc -mtriple=mips-linux-gnu -O0 -relocation-model=pic -disable-fp-elim < %s | FileCheck %s -check-prefix=PIC-FP
+
+
+; Generated using clang -target mipsel-linux-gnu -g -S test.c -emit-llvm -o test.ll
+; test.c:
+; 1: int g1 = 2;
+; 2:
+; 3: void test(int arg1) {
+; 4:   g1 = arg1;
+; 5: }
+
+
+@g1 = global i32 2, align 4
+
+; Function Attrs: nounwind
+define void @test(i32 signext %arg1) #0 {
+entry:
+  %arg1.addr = alloca i32, align 4
+  store i32 %arg1, i32* %arg1.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %arg1.addr, metadata !13, metadata !14), !dbg !15
+  %0 = load i32, i32* %arg1.addr, align 4, !dbg !16
+  store i32 %0, i32* @g1, align 4, !dbg !17
+  ret void, !dbg !18
+
+; STATIC:       addiu   $sp, $sp, -{{[0-9]+}}
+; STATIC:       sw      $4, {{[0-9]+}}($sp)
+; STATIC:       .loc    1 4 6 prologue_end
+; STATIC:       lui     ${{[0-9]+}}, %hi(g1)
+
+; STATIC-FP:    addiu   $sp, $sp, -{{[0-9]+}}
+; STATIC-FP:    sw      $fp, {{[0-9]+}}($sp)
+; STATIC-FP:    move    $fp, $sp
+; STATIC-FP:    sw      $4, {{[0-9]+}}($fp)
+; STATIC-FP:    .loc    1 4 6 prologue_end
+; STATIC-FP:    lui     ${{[0-9]+}}, %hi(g1)
+
+; PIC:          lui     $[[R0:[0-9]+]], %hi(_gp_disp)
+; PIC:          addiu   $[[R0]], $[[R0]], %lo(_gp_disp)
+; PIC:          addiu   $sp, $sp, -{{[0-9]+}}
+; PIC:          addu    $[[R1:[0-9]+]], $[[R0]], $25
+; PIC:          sw      $4, {{[0-9]+}}($sp)
+; PIC:          .loc    1 4 6 prologue_end
+; PIC:          lw      $[[R2:[0-9]+]], %got(g1)($[[R1]])
+
+; PIC-FP:       lui     $[[R0:[0-9]+]], %hi(_gp_disp)
+; PIC-FP:       addiu   $[[R0]], $[[R0]], %lo(_gp_disp)
+; PIC-FP:       addiu   $sp, $sp, -{{[0-9]+}}
+; PIC-FP:       sw      $fp, {{[0-9]+}}($sp)
+; PIC-FP:       move    $fp, $sp
+; PIC-FP:       addu    $[[R1:[0-9]+]], $[[R0]], $25
+; PIC-FP:       sw      $4, {{[0-9]+}}($fp)
+; PIC-FP:       .loc    1 4 6 prologue_end
+; PIC-FP:       lw      $[[R2:[0-9]+]], %got(g1)($[[R1]])
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind "less-precise-fpmad"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = !MDCompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 3.7.0 (trunk 232572)", isOptimized: false, runtimeVersion: 0, emissionKind: 1, enums: !2, retainedTypes: !2, subprograms: !3, globals: !8, imports: !2)
+!1 = !MDFile(filename: "test.c", directory: "/tmp")
+!2 = !{}
+!3 = !{!4}
+!4 = !MDSubprogram(name: "test", scope: !1, file: !1, line: 3, type: !5, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: false, function: void (i32)* @test, variables: !2)
+!5 = !MDSubroutineType(types: !6)
+!6 = !{null, !7}
+!7 = !MDBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!8 = !{!9}
+!9 = !MDGlobalVariable(name: "g1", scope: !0, file: !1, line: 1, type: !7, isLocal: false, isDefinition: true, variable: i32* @g1)
+!10 = !{i32 2, !"Dwarf Version", i32 4}
+!11 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = !{!"clang version 3.7.0 (trunk 232572)"}
+!13 = !MDLocalVariable(tag: DW_TAG_arg_variable, name: "arg1", arg: 1, scope: !4, file: !1, line: 3, type: !7)
+!14 = !MDExpression()
+!15 = !MDLocation(line: 3, column: 15, scope: !4)
+!16 = !MDLocation(line: 4, column: 8, scope: !4)
+!17 = !MDLocation(line: 4, column: 6, scope: !4)
+!18 = !MDLocation(line: 5, column: 1, scope: !4)