[PATCH] D92105: [RISCV] Add pre-emit pass to make more instructions compressible

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVCompressInstrs.cpp:214
+    bool DefinesTargetReg = false;
+    for (const MachineOperand &MO : MI.operands()) {
+      if (MO.isReg() && MO.getReg() == RegImm.Reg && MO.isDef()) {
----------------
Is this a strict enough check for "definition"? What about super- and sub-registers or clobbers? Would `MI.modifiesRegister` work better (and be simpler)?


================
Comment at: llvm/lib/Target/RISCV/RISCVCompressInstrs.cpp:262
+  // Update registers
+  for (MachineOperand &MO : MI.operands())
+    if (MO.isReg() && MO.getReg() == OldRegImm.Reg) {
----------------
Same question about defined...ness here.


================
Comment at: llvm/lib/Target/RISCV/RISCVCompressInstrs.cpp:278
+    MachineOperand &MOImm = MI.getOperand(2);
+    int64_t NewOffset = MOImm.getImm() & (CSW_OFFSET_MASK);
+    MOImm.setImm(NewOffset);
----------------
Nit: redundant parens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92105