[PATCH] D130192: [RISCV] Avoid redundant branch-to-branch when expanding cmpxchg
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Jul 24 17:04:34 PDT 2022
craig.topper added inline comments.
================
Comment at: llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp:531
+ return false;
+ while (MBBI != E && MBBI->isDebugInstr())
+ MBBI++;
----------------
I think this is `skipDebugInstructionsForward`?
================
Comment at: llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp:535
+ // If we have a masked cmpxchg, match AND dst, DestReg, MaskReg.
+ if (IsMasked) {
+ if (MBBI == E || MBBI->getOpcode() != RISCV::AND)
----------------
Can we avoid an `IsMasked` parameter by checking `MaskReg.isValid()`?
================
Comment at: llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp:540
+ Register ANDOp2 = MBBI->getOperand(2).getReg();
+ if (!((ANDOp1 == DestReg && ANDOp2 == MaskReg) || (ANDOp1 == MaskReg && ANDOp2 == DestReg)))
+ return false;
----------------
80 columns
================
Comment at: llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp:540
+ Register ANDOp2 = MBBI->getOperand(2).getReg();
+ if (!((ANDOp1 == DestReg && ANDOp2 == MaskReg) || (ANDOp1 == MaskReg && ANDOp2 == DestReg)))
+ return false;
----------------
craig.topper wrote:
> 80 columns
Might be a little better as
```
if (!(ANDOp1 == DestReg && ANDOp2 == MaskReg) &&
!(ANDOp1 == MaskReg && ANDOp2 == DestReg))
```
but I'm not sure.
================
Comment at: llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp:555
+ Register BNEOp1 = MBBI->getOperand(1).getReg();
+ if (!((BNEOp0 == DestReg && BNEOp1 == CmpValReg) || (BNEOp0 == CmpValReg && BNEOp1 == DestReg)))
+ return false;
----------------
80 columns
================
Comment at: llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp:555
+ Register BNEOp1 = MBBI->getOperand(1).getReg();
+ if (!((BNEOp0 == DestReg && BNEOp1 == CmpValReg) || (BNEOp0 == CmpValReg && BNEOp1 == DestReg)))
+ return false;
----------------
craig.topper wrote:
> 80 columns
Same Demorgan suggestion from above
================
Comment at: llvm/lib/Target/RISCV/RISCVExpandAtomicPseudoInsts.cpp:586
+ Register NewValReg = MI.getOperand(4).getReg();
+ Register MaskReg = IsMasked ? MI.getOperand(5).getReg() : Register(0);
+
----------------
`Register(0)` should be `Register()`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130192/new/
https://reviews.llvm.org/D130192
More information about the llvm-commits
mailing list