[PATCH] D78107: [CSInfo][MIPS] Call delay slot support in DwarfDebug

[David Stenberg via Phabricator via llvm-commits]

dstenb added a comment.

Minor comments. Looks good to me otherwise!



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:857
+      return true;
+    if (MI.isBundledWithSucc()) {
+      auto Suc = std::next(MI.getIterator());
----------------
I think it might be worth making this an early exit:

```
if (!MI.isBundledWithSucc())
  return false;
```


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:893
+      // Check if delay slot support is enabled.
+      if (!delaySlotSupported(*&MI))
         return;
----------------
I think this might read a bit as that we expect a delay slot instruction here. Could we perhaps break out the `hasDelaySlot()` check to here:

```
if (MI.hasDelaySlot() && !delaySlotSupported(*&MI))
  return;
```

or perhaps instead rename the `delaySlotSupported()` function to indicate this (I don't have any better suggestion).


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1879
+      return true;
+    if (MI.isBundledWithSucc()) {
+      auto Suc = std::next(MI.getIterator());
----------------
Same as above.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1895
       MI->isCandidateForCallSiteEntry(MachineInstr::AnyInBundle) &&
-      !MI->hasDelaySlot()) {
+      delaySlotSupported(*MI)) {
     const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
----------------
Same as above.


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

https://reviews.llvm.org/D78107