[PATCH] D122918: [RISCV][CodeGen] Support Zfinx, Zdinx, Zhinx, Zhinxmin codegen

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:53
   bool expandVRELOAD(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI);
+  bool expandpair(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI);
+  bool expandRV32_ZdinxLoad(MachineBasicBlock &MBB,
----------------
Way too generic a name that also isn't correctly capitalised


================
Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:93
+  if (MBBI->getOperand(2).isGlobal()) {
+    MBBI->getOperand(2).setOffset(4);
+    BuildMI(MBB, MBBI, DL, TII->get(RISCV::SW))
----------------
Is that guaranteed to be 0?


================
Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:102
+        .add(MBBI->getOperand(1))
+        .addImm(MBBI->getOperand(2).getImm() + 4);
+  }
----------------
Think about whether this is actually safe, please...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122918