[PATCH] D131485: [AMDGPU] Fix prologue/epilogue markers in .debug_line table for trivial functions

[Scott Linder via Phabricator via llvm-commits]

scott.linder added a comment.

One nit, but the rest LGTM



================
Comment at: llvm/lib/Target/AMDGPU/SIFrameLowering.cpp:974
+  // hits at the beginning of the epilogue.
+  DebugLoc DL = MBBI != MBB.end() ? MBBI->getDebugLoc() : DebugLoc();
 
----------------
Looking at other targets, I'm not sure how to interpret the case where there are no terminators. Some backends seem to assume there is always a terminator:
```
void MSP430FrameLowering::emitEpilogue(MachineFunction &MF,
                                       MachineBasicBlock &MBB) const {
  ...
  DebugLoc DL = MBBI->getDebugLoc();
```

Others also explicitly handle the case where `MBB.empty()`, and also use the `DebugLoc` of the last non-debug instruction instead of just using an empty `DebugLoc()`:
```
void CSKYFrameLowering::emitEpilogue(MachineFunction &MF,
                                     MachineBasicBlock &MBB) const {
  ...
  // Get the insert location for the epilogue. If there were no terminators in
  // the block, get the last instruction.
  MachineBasicBlock::iterator MBBI = MBB.end();
  DebugLoc DL;
  if (!MBB.empty()) {
    MBBI = MBB.getFirstTerminator();
    if (MBBI == MBB.end())
      MBBI = MBB.getLastNonDebugInstr();
    DL = MBBI->getDebugLoc();

    // If this is not a terminator, the actual insert location should be after
    // the last instruction.
    if (!MBBI->isTerminator())
      MBBI = std::next(MBBI);
  }
```

There may be other approaches, I didn't review every Target's impl, but I think we should pick one of the existing approaches unless we have a strong reason not to.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131485/new/

https://reviews.llvm.org/D131485