[llvm] a21bbc2 - [MachineCombiner][RISCV] Make hasReassociableSibling virtual and override it for RISCV

[Anton Sidorenko via llvm-commits]

Author: Anton Sidorenko
Date: 2022-12-01T16:30:51+03:00
New Revision: a21bbc24d306892136240f31e68c607775a6d7a3

URL: https://github.com/llvm/llvm-project/commit/a21bbc24d306892136240f31e68c607775a6d7a3
DIFF: https://github.com/llvm/llvm-project/commit/a21bbc24d306892136240f31e68c607775a6d7a3.diff

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

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.

Differential Revision: https://reviews.llvm.org/D138302

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/TargetInstrInfo.h
    llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
    llvm/lib/Target/RISCV/RISCVInstrInfo.h

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/TargetInstrInfo.h b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
index 189db64609a48..f36a5d394caff 100644
--- a/llvm/include/llvm/CodeGen/TargetInstrInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetInstrInfo.h
@@ -1180,7 +1180,8 @@ class TargetInstrInfo : public MCInstrInfo {
                                        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

diff  --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index c1090e6e164a4..d43bf24129593 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -1199,28 +1199,26 @@ static bool isFMUL(unsigned Opc) {
   }
 }
 
-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,
@@ -1250,23 +1248,6 @@ static bool canCombineFPFusedMultiply(const MachineInstr &Root,
   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,
@@ -1294,10 +1275,7 @@ getFPFusedMultiplyPatterns(MachineInstr &Root,
 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(

diff  --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index 3eaca1e781b95..90a9b5a9a6214 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -202,6 +202,11 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
       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;
 };