[PATCH] D138302: [MachineCombiner][RISCV] Make hasReassociableSibling virtual and override it for RISCV

[Anton Sidorenko via Phabricator via llvm-commits]

asi-sc added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:1211
 
-  if (!Root.getFlag(MachineInstr::MIFlag::FmReassoc) ||
-      !Root.getFlag(MachineInstr::MIFlag::FmNsz) ||
-      !MI->getFlag(MachineInstr::MIFlag::FmReassoc) ||
-      !MI->getFlag(MachineInstr::MIFlag::FmNsz))
-    return false;
+  return RISCV::hasEqualFRM(Inst, Sibling);
+}
----------------
Currently, `hasEqualFRM` returns `false` even if both instructions don't have FRM at all. So, to add integer instructions reassociation we:
  1. Can check that `Inst` and `Sibling` are floating-point instructions and check FRM equality only in this case 
  Or
  2. return `true` from `hasEqualFRM` if both instructions do not have FRM operand.

I'd like to ask reviewers which way is better to follow? I'd prefer the second option. 

The question is not about this review, but I'm trying to think a few steps ahead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138302