[mips] Expand on r206218 to put the prologue_end marker in the correct place.

[dsanders]

Summary

The difficulty with moving the prologue_end marker turned out to be that the
stack decrement instruction and all the instructions leading up to it
(including CFI_INSTRUCTIONs) needed to be marked with the FrameSetup flag as
well.

I'll fix MIPS16 in a follow-up.

[dsanders]

Adding a comment so that phabricator posts this patch to the list.

[echristo]

Hrm. Looks like most targets are going to end up needing this, but there are a few complications. I'd almost rather just add it during the instruction build rather than iterating through all of the instructions as post-processing.

Thoughts?

[dsanders]

I started off adding it to the construction of each instruction but I switched to the post-processing method after noticing the loadImmediate() call in adjustStackPtr(). I was a bit reluctant to add an argument to a general function to deal with a special case needed by one caller. The same applies to storeRegToStackSlot().

It's also slightly more convenient to implement as a post-process step but that's not a very good reason to do it that way since emitPrologue() is rarely changed once it's implemented.

lib/Target/Mips/MipsSEFrameLowering.cpp

360–361

The storeRegToStackSlot() call

lib/Target/Mips/MipsSEInstrInfo.cpp

372

This is the loadImmediate() call

376–382

I just noticed that the loop in emitPrologue() should cover a superset of the instructions that this loop covers. I'll have to try removing this loop to see if it makes a difference. Also, I've left some debugging output here which I'll remove.

[echristo]

How about we move this to PEI::insertPrologEpilogCode instead and make it work for all targets and then we won't have to worry about annotating instructions?

-eric

[dsanders]

That sounds good to me. I'll try to fit in updating my patch next week

[echristo]

BTW I looked at this a while ago and it's a bit more complicated than
just moving it up so adding this patch in to the mips port (and, if
you don't mind a couple other ports) is probably the right way to go
for now.

-eric

[dsanders]

Abandoning a very old patch.

lib/Target/Mips/MipsSEFrameLowering.cpp

void MipsSEFrameLowering::emitPrologue(MachineFunction &MF) const {
   if (MipsFI->callsEhReturn()) {
     const TargetRegisterClass *RC = STI.isABI_N64() ?
         &Mips::GPR64RegClass : &Mips::GPR32RegClass;
 
     // Insert instructions that spill eh data registers.
     for (int I = 0; I < 4; ++I) {
       if (!MBB.isLiveIn(ehDataReg(I)))
         MBB.addLiveIn(ehDataReg(I));
       TII.storeRegToStackSlot(MBB, MBBI, ehDataReg(I), false,
                               MipsFI->getEhDataRegFI(I), RC, &RegInfo);
     }
 
     // Emit .cfi_offset directives for eh data registers.
     for (int I = 0; I < 4; ++I) {
       int64_t Offset = MFI->getObjectOffset(MipsFI->getEhDataRegFI(I));
       unsigned Reg = MRI->getDwarfRegNum(ehDataReg(I), true);
       unsigned CFIIndex = MMI.addFrameInst(
           MCCFIInstruction::createOffset(nullptr, Reg, Offset));
BuildMI(MBB, MBBI, dl, TII.get(ADDu), FP).addReg(SP).addReg(ZERO)
       .setMIFlag(MachineInstr::FrameSetup);
 
     // emit ".cfi_def_cfa_register $fp"
     unsigned CFIIndex = MMI.addFrameInst(MCCFIInstruction::createDefCfaRegister(
         nullptr, MRI->getDwarfRegNum(FP, true)));
     BuildMI(MBB, MBBI, dl, TII.get(TargetOpcode::CFI_INSTRUCTION))
         .addCFIIndex(CFIIndex);
   }
+
+  // All instructions up to this point are frame setup instructions
+  // This isn't expected to touch any instructions this function didn't create
+  // but it's fine if it does.
+  for (MachineBasicBlock::iterator MI = MBB.begin(); MI != MBBI; ++MI)
+    MI->setFlag(MachineInstr::FrameSetup);
 }
 
 void MipsSEFrameLowering::emitEpilogue(MachineFunction &MF,
                                        MachineBasicBlock &MBB) const {
   MachineBasicBlock::iterator MBBI = MBB.getLastNonDebugInstr();
   MachineFrameInfo *MFI            = MF.getFrameInfo();
   MipsFunctionInfo *MipsFI = MF.getInfo<MipsFunctionInfo>();
 

lib/Target/Mips/MipsSEInstrInfo.cpp

 #include "MipsSEInstrInfo.h"
 #include "InstPrinter/MipsInstPrinter.h"
 #include "MipsMachineFunction.h"
 #include "MipsTargetMachine.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/TargetRegistry.h"
 
 using namespace llvm;
 
 MipsSEInstrInfo::MipsSEInstrInfo(MipsTargetMachine &tm)
   : MipsInstrInfo(tm,
                   tm.getRelocationModel() == Reloc::PIC_ ? Mips::B : Mips::J),
void MipsSEInstrInfo::adjustStackPtr(unsigned SP, int64_t Amount,
   const MipsSubtarget &STI = TM.getSubtarget<MipsSubtarget>();
   DebugLoc DL = I != MBB.end() ? I->getDebugLoc() : DebugLoc();
   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);
   }
+
+  // Mark stack decrements as being part of a frame setup.
+  if (Amount < 0) {
+    for (MachineBasicBlock::iterator MI = MBB.begin(); MI != I; ++MI) {
+      errs() << "Flagging "; MI->print(errs()); errs() << "\n";
+      MI->setFlag(MachineInstr::FrameSetup);
+    }
+  }
 }
 
 /// This function generates the sequence of instructions needed to get the
 /// result of adding register REG and immediate IMM.
 unsigned
 MipsSEInstrInfo::loadImmediate(int64_t Imm, MachineBasicBlock &MBB,
                                MachineBasicBlock::iterator II, DebugLoc DL,
                                unsigned *NewImm) const {

test/DebugInfo/Mips/lit.local.cfg

+targets = set(config.root.targets_to_build.split())
+if not 'Mips' in targets:
+    config.unsupported = True
+

test/DebugInfo/Mips/unconditional-branch.ll

+; REQUIRES: object-emission
+; PR 19261
+
+; This test is derived from test/DebugInfo/unconditional-branch.ll and has been
+; made a bit stricter to test the prologue_end marker.
+
+; RUN: %llc_dwarf -march=mips -fast-isel=false -O0 -filetype=obj %s -o %t
+; RUN: llvm-dwarfdump %t | FileCheck %s
+
+; CHECK: 0x0000000000000000      1      0      1   0             0  is_stmt
+
+; This next address probably ought to be 0xc instead of 0x10. The store to
+; %i.addr is incorrectly included in the prologue because it lacks source
+; location information. However, the current behaviour is correct for the
+; metadata provided in the test.
+; CHECK: 0x0000000000000010      2      0      1   0             0  is_stmt prologue_end
+
+; CHECK: 0x0000000000000018      4      0      1   0             0  is_stmt
+
+; IR generated from clang -O0 -g with the following source:
+;void foo(int i){
+;  switch(i){
+;  default:
+;    break;
+;  }
+;  return;
+;}
+
+; Function Attrs: nounwind
+define void @foo(i32 %i) #0 {
+entry:
+  %i.addr = alloca i32, align 4
+  store i32 %i, i32* %i.addr, align 4
+  call void @llvm.dbg.declare(metadata !{i32* %i.addr}, metadata !12), !dbg !13
+  %0 = load i32* %i.addr, align 4, !dbg !14
+  switch i32 %0, label %sw.default [
+  ], !dbg !14
+
+sw.default:                                       ; preds = %entry
+  br label %sw.epilog, !dbg !15
+
+sw.epilog:                                        ; preds = %sw.default
+  ret void, !dbg !17
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata) #1
+
+attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "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 = !{!9, !10}
+!llvm.ident = !{!11}
+
+!0 = metadata !{i32 786449, metadata !1, i32 12, metadata !"clang version 3.5.0 (204712)", i1 false, metadata !"", i32 0, metadata !2, metadata !2, metadata !3, metadata !2, metadata !2, metadata !"", i32 1} ; [ DW_TAG_compile_unit ] [D:\work\EPRs\396363/test.c] [DW_LANG_C99]
+!1 = metadata !{metadata !"test.c", metadata !"D:\5Cwork\5CEPRs\5C396363"}
+!2 = metadata !{}
+!3 = metadata !{metadata !4}
+!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"foo", metadata !"foo", metadata !"", i32 1, metadata !6, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 false, void (i32)* @foo, null, null, metadata !2, i32 1} ; [ DW_TAG_subprogram ] [line 1] [def] [foo]
+!5 = metadata !{i32 786473, metadata !1}          ; [ DW_TAG_file_type ] [D:\work\EPRs\396363/test.c]
+!6 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !7, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!7 = metadata !{null, metadata !8}
+!8 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64 32, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]
+!9 = metadata !{i32 2, metadata !"Dwarf Version", i32 4}
+!10 = metadata !{i32 1, metadata !"Debug Info Version", i32 1}
+!11 = metadata !{metadata !"clang version 3.5.0 (204712)"}
+!12 = metadata !{i32 786689, metadata !4, metadata !"i", metadata !5, i32 16777217, metadata !8, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [i] [line 1]
+!13 = metadata !{i32 1, i32 0, metadata !4, null}
+!14 = metadata !{i32 2, i32 0, metadata !4, null}
+!15 = metadata !{i32 4, i32 0, metadata !16, null}
+!16 = metadata !{i32 786443, metadata !1, metadata !4, i32 2, i32 0, i32 0, i32 0} ; [ DW_TAG_lexical_block ] [D:\work\EPRs\396363/test.c]
+!17 = metadata !{i32 6, i32 0, metadata !4, null}