[PATCH] D126932: [RISCV] Reduce scalar load/store isel patterns to a single ComplexPattern. NFCI
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 3 08:48:45 PDT 2022
craig.topper added inline comments.
================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:1920
+ // We might be able to treat this OR as an ADD.
+ if (auto *FIN = dyn_cast<FrameIndexSDNode>(Addr.getOperand(0))) {
+ if (auto *CN = dyn_cast<ConstantSDNode>(Addr.getOperand(1))) {
----------------
reames wrote:
> craig.topper wrote:
> > reames wrote:
> > > Why does this case need to be restricted to frame index? I think you maybe copy pasted this and didn't adjust?
> > This was an attempt to preserve the original isel pattern. isOrEquivalentToAdd only handles frame index and the isel pattern used AddrFI.
> >
> > I'm considering using the more powerful SelectionDAG::isBaseWithConstantOffset that uses computeKnownBits for OR instead of special casing FrameIndex.
> Let's do that as a follow up. That way the code changes end up in their own review separate from what is otherwise mostly NFC.
>
> Add a comment to that effect in this patch please.
I wrote the patch to use SelectionDAG::isBaseWithConstantOffset and it does cause test changes to vararg.ll.
================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1207
-def : Pat<(add (XLenVT AddrFI:$Rs), simm12:$imm12),
- (ADDI (XLenVT AddrFI:$Rs), simm12:$imm12)>;
-def : Pat<(IsOrAdd (XLenVT AddrFI:$Rs), simm12:$imm12),
- (ADDI (XLenVT AddrFI:$Rs), simm12:$imm12)>;
+def : Pat<(FrameAddrRegImm GPR:$rs1, simm12:$imm12),
+ (ADDI GPR:$rs1, simm12:$imm12)>;
----------------
reames wrote:
> Do we even need a separate FrameAddRegImm here? The addressing mode should be the same for FI vs non-FI.
>
> Can we write something along the lines of AddRegImm AddrFI:$Rs, simm12:$imm12? Not sure of my tablegen syntax here.
ComplexPatterns can't nest so AddRegImm AddrFI:$Rs, simm12:$imm12 wouldn't work.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126932/new/
https://reviews.llvm.org/D126932
More information about the llvm-commits
mailing list