[MachineCombiner][RISCV] Make hasReassociableSibling virtual and override it for RISCV

[asi-sc]

Summary

To check reassociation correctness for RISCV, we must ensure that the root and
it's sibling have equal rounding modes (for floating point instructions).
hasReassociableSibling is a good place to make additional target-dependend
checks.

This patch allows us to enable default machine combiner mechanism to gather
reassociation candidates on RISCV.

[asi-sc]

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

1212–1213

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.

[craig.topper]

LGTM

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

1212–1213

Right now hasEqualFRM returns false if either instruction doesn't have an FRM operand. Does that false case ever happen? Sounds like your option 2 here would need that to return true if the FRM operand isn't present.

[asi-sc]

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

1212–1213

I spent some time thinking about this and changed my opinion. Actually, checking floating-point rounding modes of integer instructions is a strange idea. So, I'll keep the existing behavior.

llvm/include/llvm/CodeGen/TargetInstrInfo.h

virtual bool isAssociativeAndCommutative(const MachineInstr &Inst) const {
     return false;
   }
 
   /// Return true when \P Inst has reassociable operands in the same \P MBB.
   virtual bool hasReassociableOperands(const MachineInstr &Inst,
                                        const MachineBasicBlock *MBB) const;
 
   /// Return true when \P Inst has reassociable sibling.
-  bool hasReassociableSibling(const MachineInstr &Inst, bool &Commuted) const;
+  virtual bool hasReassociableSibling(const MachineInstr &Inst,
+                                      bool &Commuted) const;
 
   /// When getMachineCombinerPatterns() finds patterns, this function generates
   /// the instructions that could replace the original code sequence. The client
   /// has to decide whether the actual replacement is beneficial or not.
   /// \param Root - Instruction that could be combined with one of its operands
   /// \param Pattern - Combination pattern for Root
   /// \param InsInstrs - Vector of new instructions that implement P
   /// \param DelInstrs - Old instructions, including Root, that could be

llvm/lib/Target/RISCV/RISCVInstrInfo.h

finalizeInsInstrs(MachineInstr &Root, MachineCombinerPattern &P,
                     SmallVectorImpl<MachineInstr *> &InsInstrs) const override;
 
   void genAlternativeCodeSequence(
       MachineInstr &Root, MachineCombinerPattern Pattern,
       SmallVectorImpl<MachineInstr *> &InsInstrs,
       SmallVectorImpl<MachineInstr *> &DelInstrs,
       DenseMap<unsigned, unsigned> &InstrIdxForVirtReg) const override;
 
+  bool hasReassociableSibling(const MachineInstr &Inst,
+                              bool &Commuted) const override;
+
+  bool isAssociativeAndCommutative(const MachineInstr &Inst) const override;
+
 protected:
   const RISCVSubtarget &STI;
 };
 
 namespace RISCV {
 
 // Returns true if this is the sext.w pattern, addiw rd, rs1, 0.
 bool isSEXT_W(const MachineInstr &MI);

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

default:
     return false;
   case RISCV::FMUL_H:
   case RISCV::FMUL_S:
   case RISCV::FMUL_D:
     return true;
   }
 }
 
-static bool isAssociativeAndCommutativeFPOpcode(unsigned Opc) {
-  return isFADD(Opc) || isFMUL(Opc);
-}
-
-static bool canReassociate(MachineInstr &Root, MachineOperand &MO) {
-  if (!MO.isReg() || !Register::isVirtualRegister(MO.getReg()))
-    return false;
-  MachineRegisterInfo &MRI = Root.getMF()->getRegInfo();
-  MachineInstr *MI = MRI.getVRegDef(MO.getReg());
-  if (!MI || !MRI.hasOneNonDBGUse(MO.getReg()))
+bool RISCVInstrInfo::hasReassociableSibling(const MachineInstr &Inst,
+                                            bool &Commuted) const {
+  if (!TargetInstrInfo::hasReassociableSibling(Inst, Commuted))
     return false;
 
-  if (MI->getOpcode() != Root.getOpcode())
-    return false;
+  const MachineRegisterInfo &MRI = Inst.getMF()->getRegInfo();
+  unsigned OperandIdx = Commuted ? 2 : 1;
+  const MachineInstr &Sibling =
+      *MRI.getVRegDef(Inst.getOperand(OperandIdx).getReg());
 
-  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);
+}
 
-  return RISCV::hasEqualFRM(Root, *MI);
+bool RISCVInstrInfo::isAssociativeAndCommutative(
+    const MachineInstr &Inst) const {
+  unsigned Opc = Inst.getOpcode();
+  if (isFADD(Opc) || isFMUL(Opc))
+    return Inst.getFlag(MachineInstr::MIFlag::FmReassoc) &&
+           Inst.getFlag(MachineInstr::MIFlag::FmNsz);
+  return false;
 }
 
 static bool canCombineFPFusedMultiply(const MachineInstr &Root,
                                       const MachineOperand &MO,
                                       bool DoRegPressureReduce) {
   if (!MO.isReg() || !Register::isVirtualRegister(MO.getReg()))
     return false;
   const MachineRegisterInfo &MRI = Root.getMF()->getRegInfo();
static bool canCombineFPFusedMultiply(const MachineInstr &Root,
 
   // Do not combine instructions from different basic blocks.
   if (Root.getParent() != MI->getParent())
     return false;
   return RISCV::hasEqualFRM(Root, *MI);
 }
 
 static bool
-getFPReassocPatterns(MachineInstr &Root,
-                     SmallVectorImpl<MachineCombinerPattern> &Patterns) {
-  bool Added = false;
-  if (canReassociate(Root, Root.getOperand(1))) {
-    Patterns.push_back(MachineCombinerPattern::REASSOC_AX_BY);
-    Patterns.push_back(MachineCombinerPattern::REASSOC_XA_BY);
-    Added = true;
-  }
-  if (canReassociate(Root, Root.getOperand(2))) {
-    Patterns.push_back(MachineCombinerPattern::REASSOC_AX_YB);
-    Patterns.push_back(MachineCombinerPattern::REASSOC_XA_YB);
-    Added = true;
-  }
-  return Added;
-}
-
-static bool
 getFPFusedMultiplyPatterns(MachineInstr &Root,
                            SmallVectorImpl<MachineCombinerPattern> &Patterns,
                            bool DoRegPressureReduce) {
   unsigned Opc = Root.getOpcode();
   bool IsFAdd = isFADD(Opc);
   if (!IsFAdd && !isFSUB(Opc))
     return false;
   bool Added = false;
if (canCombineFPFusedMultiply(Root, Root.getOperand(2),
     Added = true;
   }
   return Added;
 }
 
 static bool getFPPatterns(MachineInstr &Root,
                           SmallVectorImpl<MachineCombinerPattern> &Patterns,
                           bool DoRegPressureReduce) {
-  bool Added = getFPFusedMultiplyPatterns(Root, Patterns, DoRegPressureReduce);
-  if (isAssociativeAndCommutativeFPOpcode(Root.getOpcode()))
-    Added |= getFPReassocPatterns(Root, Patterns);
-  return Added;
+  return getFPFusedMultiplyPatterns(Root, Patterns, DoRegPressureReduce);
 }
 
 bool RISCVInstrInfo::getMachineCombinerPatterns(
     MachineInstr &Root, SmallVectorImpl<MachineCombinerPattern> &Patterns,
     bool DoRegPressureReduce) const {
 
   if (getFPPatterns(Root, Patterns, DoRegPressureReduce))
     return true;