[PATCH] D144249: [RISCV] Add vendor-defined XTheadMemIdx (Indexed Memory Operations) extension

[Manolis Tsamis via Phabricator via llvm-commits]

mtsamis marked 5 inline comments as done.
mtsamis added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2224
+      Index = N.getOperand(0);
+      ShiftAmt = N.getConstantOperandVal(1);
+    } else {
----------------
craig.topper wrote:
> Do we need to check the shift amount is 1,2, or 3?
I wanted to make this pattern generic and not limited to specific shift amounts.
For the specific MemIdx extension the shift amount should be enforced on the pattern side with `(AddrRegRegScale GPR:$rs1, GPR:$rs2, uimm2:$uimm2)`.
If I'm not mistaken, SelectAddrRegRegScale can match any shift amount but that will be rejected by the uimm2 there.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td:689
+  ValueType vt = XLenVT> {
+def : Pat<(st (vt GPR:$rd), GPR:$rs1, simm5:$off),
+          (Inst GPR:$rd, GPR:$rs1, simm5:$off, 0)>;
----------------
craig.topper wrote:
> We might look at a ComplexPattern here to match a shifted simm5 and return the simm5 and the shift amount as 2 results.
Sounds good, I'll try to refactor that with a ComplexPattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144249