[PATCH] D78107: [CSInfo][MIPS] Call delay slot support in DwarfDebug
David Stenberg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 25 00:29:40 PDT 2020
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
More information about the llvm-commits
mailing list