[PATCH] D144002: [RISCV] Add vendor-defined XTheadMemPair (two-GPR Memory Operations) extension

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2665
+      SMLoc Loc = Operands[1]->getStartLoc();
+      return Error(Loc, "The source register and destination registers "
+                        "cannot be equal.");
----------------
Does this restrict apply for TH_SDD/TH_SWD? I don't see it in the spec.


================
Comment at: llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp:454
+                   Opcode == RISCV::TH_SWD);
+  if (IsWordOp) {
+    Inst.addOperand(MCOperand::createImm(3));
----------------
Drop curly braces


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:9744
+  MachineMemOperand *NewMMO = MF.getMachineMemOperand(
+      MMO, MMO->getPointerInfo(), MemVT == MVT::i32 ? 8 : 16);
+
----------------
Since the offset allows for a gap in the memory locations the size isn't 8/16. It needs to be UnknownSize or large enough to cover the gap.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:9803
+  auto ExtractBaseAndOffset = [](SDValue Ptr) -> std::pair<SDValue, uint64_t> {
+    if (Ptr->getOpcode() == ISD::ADD) {
+      if (auto *C1 = dyn_cast<ConstantSDNode>(Ptr->getOperand(1))) {
----------------
Drop curly braces


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:9842
+      if (MemVT == MVT::i32) {
+        // Check for adjucent i32 values and a 2-bit index.
+        if ((Offset1 + 4 != Offset2) || !isShiftedUInt<2, 3>(Offset1))
----------------
adjacent*


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:9846
+      } else {
+        // Check for adjucent i64 values and a 2-bit index.
+        if ((Offset1 + 8 != Offset2) || !isShiftedUInt<2, 4>(Offset1))
----------------
adjacent*


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:9847
+        // Check for adjucent i64 values and a 2-bit index.
+        if ((Offset1 + 8 != Offset2) || !isShiftedUInt<2, 4>(Offset1))
+          continue;
----------------
How do we know MemVT is i64 here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144002