[PATCH] D135264: [MachineCombiner][RISCV] Enable MachineCombiner for RISCV

[Anton Sidorenko via Phabricator via llvm-commits]

asi-sc added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:1185
+static bool
+getFPReassocPatterns(MachineInstr &Root,
+                     SmallVectorImpl<MachineCombinerPattern> &Patterns) {
----------------
I'd like to comment why I don't follow the default path here (`TargetInstrInfo::getMachineCombinerPatterns -> TargetInstrInfo::isReassociationCandidate -> ...`). There are two main reasons:
* Generic implementation relies on `isAssociativeAndCommutative` function that takes MachineInstruction. Having it, we can check the opcode, fast math flags, etc. However, instructions are checked independently, so it's not possible to test if two instructions have equal rounding modes. Adding one more interface method required only by RISCV target doesn't seem to be a good idea.
* If we decide to introduce machine combiner for RISCV, I'd like to add patterns for FSUB instruction as well, e.g. `(X + A) - Y => (X - Y) + A`. These FSUB patterns need approximately the same checks as the patterns from this patch but cannot follow default (associative and commutative) path. It means that this code will be required anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135264