[MIPatternMatch]: Add matchers for binary instructions

[Petar.Avramovic]

Summary

Add matchers that support commutative and non-commutative binary opcodes.

[paquette]

Testcase?

llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h

258

Commutative?

268

Maybe a bit simpler to change this to the negative case?

Also may want to check that there is only one def.

279

322

[Petar.Avramovic]

Simplify tests that fail because of non-binary instruction (match register instead of m_ICst) since they fail when number of def/use operands is checked. Update commit message.

[foad]

I'm not sure the "AnyBinaryOp" versions are really needed for anything -- see https://reviews.llvm.org/D90050#inline-955365.

As for BinaryOpWithOpcode_match, couldn't you just change the existing BinaryOp_match to take the opcode as a regular argument instead of a template argument? (Are you worried about making it slower? Hopefully with proper inlining there would be no codegen difference at all.)

[Petar.Avramovic]

Removed matchers that don't check opcode.
My guess is that this was originally done as a equivalent of IR/PatternMatch.h.
Thus i also added AnyBinaryOp_match alongside with the version that checks opcode which i originally wanted(named BinaryOpc_match here).
I thought that AnyBinaryOp_match would be useful when we know opcode already so we avoid checking it again, but turns out that at the moment this is not that useful.
I would prefer to leave existing code with opcodes as template arguments like in IR/PatternMatch.h and add new matcher that has opcode as an regular argument instead. m_Add ect. are meant to have templated opcode and changing it to regular opcode argument almost guaranteed results in a same thing but brings no improvement.

[foad]

I would prefer to leave existing code with opcodes as template arguments like in IR/PatternMatch.h and add new matcher that has opcode as an regular argument instead.

Fair enough, if no-one else objects to that approach.

llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h

if (mi_match(Op, MRI, m_MInstr(TmpMI))) {
                (Commutable && (R.match(MRI, TmpMI->getOperand(1).getReg()) &&
                                L.match(MRI, TmpMI->getOperand(2).getReg())));
       }
     }
     return false;
   }
 };
 
+// Helper for (commutative) binary generic MI that checks Opcode.
+template <typename LHS_P, typename RHS_P, bool Commutable = false>
+struct BinaryOpc_match {
+  unsigned Opc;
+  LHS_P L;
+  RHS_P R;
+
+  BinaryOpc_match(unsigned Opcode, const LHS_P &LHS, const RHS_P &RHS)
+      : Opc(Opcode), L(LHS), R(RHS) {}
+  template <typename OpTy>
+  bool match(const MachineRegisterInfo &MRI, OpTy &&Op) {
+    MachineInstr *TmpMI;
+    if (mi_match(Op, MRI, m_MInstr(TmpMI))) {
+      if (TmpMI->getOpcode() == Opc && TmpMI->getNumDefs() == 1 &&
+          TmpMI->getNumOperands() == 3) {
+        return (L.match(MRI, TmpMI->getOperand(1).getReg()) &&
+                R.match(MRI, TmpMI->getOperand(2).getReg())) ||
+               (Commutable && (R.match(MRI, TmpMI->getOperand(1).getReg()) &&
+                               L.match(MRI, TmpMI->getOperand(2).getReg())));
+      }
+    }
+    return false;
+  }
+};
+
+template <typename LHS, typename RHS>
+inline BinaryOpc_match<LHS, RHS, false> m_BinOp(unsigned Opcode, const LHS &L,
+                                                const RHS &R) {
+  return BinaryOpc_match<LHS, RHS, false>(Opcode, L, R);
+}
+
+template <typename LHS, typename RHS>
+inline BinaryOpc_match<LHS, RHS, true>
+m_CommutativeBinOp(unsigned Opcode, const LHS &L, const RHS &R) {
+  return BinaryOpc_match<LHS, RHS, true>(Opcode, L, R);
+}
+
 template <typename LHS, typename RHS>
 inline BinaryOp_match<LHS, RHS, TargetOpcode::G_ADD, true>
 m_GAdd(const LHS &L, const RHS &R) {
   return BinaryOp_match<LHS, RHS, TargetOpcode::G_ADD, true>(L, R);
 }
 
 template <typename LHS, typename RHS>
 inline BinaryOp_match<LHS, RHS, TargetOpcode::G_PTR_ADD, true>

 inline BinaryOp_match<LHS, RHS, TargetOpcode::G_MUL, true>
 m_GMul(const LHS &L, const RHS &R) {
   return BinaryOp_match<LHS, RHS, TargetOpcode::G_MUL, true>(L, R);
 }
 
 template <typename LHS, typename RHS>
 inline BinaryOp_match<LHS, RHS, TargetOpcode::G_FADD, true>
 m_GFAdd(const LHS &L, const RHS &R) {
   return BinaryOp_match<LHS, RHS, TargetOpcode::G_FADD, true>(L, R);
 }
 
 template <typename LHS, typename RHS>
 inline BinaryOp_match<LHS, RHS, TargetOpcode::G_FMUL, true>
 m_GFMul(const LHS &L, const RHS &R) {
   return BinaryOp_match<LHS, RHS, TargetOpcode::G_FMUL, true>(L, R);
 }
 

llvm/unittests/CodeGen/GlobalISel/PatternMatchTest.cpp

TEST_F(AArch64GISelMITest, MatchBinaryOp) {
   EXPECT_EQ(Src1, TruncCopy1.getReg(0));
 
   // Build a G_PTR_ADD and check that we can match it.
   auto PtrAdd = B.buildPtrAdd(p0, {B.buildUndef(p0)}, Copies[0]);
   match = mi_match(PtrAdd.getReg(0), *MRI, m_GPtrAdd(m_Reg(Src0), m_Reg(Src1)));
   EXPECT_TRUE(match);
   EXPECT_EQ(Src0, PtrAdd->getOperand(1).getReg());
   EXPECT_EQ(Src1, Copies[0]);
+
+  auto MIBCst = B.buildConstant(s64, 42);
+  auto MIBAddCst = B.buildAdd(s64, MIBCst, Copies[0]);
+  auto MIBUnmerge = B.buildUnmerge({s32, s32}, B.buildConstant(s64, 42));
+
+  // m_BinOp with opcode.
+  // Match binary instruction, opcode and its non-commutative operands.
+  match = mi_match(MIBAddCst, *MRI,
+                   m_BinOp(TargetOpcode::G_ADD, m_ICst(Cst), m_Reg(Src0)));
+  EXPECT_TRUE(match);
+  EXPECT_EQ(Src0, Copies[0]);
+  EXPECT_EQ(Cst, 42);
+
+  // Opcode doesn't match.
+  match = mi_match(MIBAddCst, *MRI,
+                   m_BinOp(TargetOpcode::G_MUL, m_ICst(Cst), m_Reg(Src0)));
+  EXPECT_FALSE(match);
+
+  match = mi_match(MIBAddCst, *MRI,
+                   m_BinOp(TargetOpcode::G_ADD, m_Reg(Src0), m_ICst(Cst)));
+  EXPECT_FALSE(match);
+
+  // Instruction is not binary.
+  match = mi_match(MIBCst, *MRI,
+                   m_BinOp(TargetOpcode::G_MUL, m_Reg(Src0), m_Reg(Src1)));
+  EXPECT_FALSE(match);
+  match = mi_match(MIBUnmerge, *MRI,
+                   m_BinOp(TargetOpcode::G_MUL, m_Reg(Src0), m_Reg(Src1)));
+  EXPECT_FALSE(match);
+
+  // m_CommutativeBinOp with opcode.
+  match = mi_match(
+      MIBAddCst, *MRI,
+      m_CommutativeBinOp(TargetOpcode::G_ADD, m_ICst(Cst), m_Reg(Src0)));
+  EXPECT_TRUE(match);
+  EXPECT_EQ(Src0, Copies[0]);
+  EXPECT_EQ(Cst, 42);
+
+  match = mi_match(
+      MIBAddCst, *MRI,
+      m_CommutativeBinOp(TargetOpcode::G_MUL, m_ICst(Cst), m_Reg(Src0)));
+  EXPECT_FALSE(match);
+
+  match = mi_match(
+      MIBAddCst, *MRI,
+      m_CommutativeBinOp(TargetOpcode::G_ADD, m_Reg(Src0), m_ICst(Cst)));
+  EXPECT_TRUE(match);
+  EXPECT_EQ(Src0, Copies[0]);
+  EXPECT_EQ(Cst, 42);
+
+  match = mi_match(
+      MIBCst, *MRI,
+      m_CommutativeBinOp(TargetOpcode::G_MUL, m_Reg(Src0), m_Reg(Src1)));
+  EXPECT_FALSE(match);
+  match = mi_match(
+      MIBUnmerge, *MRI,
+      m_CommutativeBinOp(TargetOpcode::G_MUL, m_Reg(Src0), m_Reg(Src1)));
+  EXPECT_FALSE(match);
 }
 
 TEST_F(AArch64GISelMITest, MatchICmp) {
   setUp();
   if (!TM)
     return;
 
   const LLT s1 = LLT::scalar(1);