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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp:2224
+      Index = N.getOperand(0);
+      ShiftAmt = N.getConstantOperandVal(1);
+    } else {
----------------
mtsamis wrote:
> 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.
I worry the ComplexPattern might have bypassed that check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144249