[globalisel][tablegen] Revise API for ComplexPattern operands to improve flexibility.

Summary:
Some targets need to be able to do more complex rendering than just adding an
operand or two to an instruction. For example, it may need to insert an
instruction to extract a subreg first, or it may need to perform an operation
on the operand.

In SelectionDAG, targets would create SDNode's to achieve the desired effect
during the complex pattern predicate. This worked because SelectionDAG had a
form of garbage collection that would take care of SDNode's that were created
but not used due to a later predicate rejecting a match. This doesn't translate
well to GlobalISel and the churn was wasteful.

The API changes in this patch enable GlobalISel to accomplish the same thing
without the waste. The API is now:
	InstructionSelector::OptionalComplexRendererFn selectArithImmed(MachineOperand &Root) const;
where Root is the root of the match. The return value can be omitted to
indicate that the predicate failed to match, or a function with the signature
ComplexRendererFn can be returned. For example:
	return OptionalComplexRendererFn(
	       [=](MachineInstrBuilder &MIB) { MIB.addImm(Immed).addImm(ShVal); });
adds two immediate operands to the rendered instruction. Immed and ShVal are
captured from the predicate function.

As an added bonus, this also reduces the amount of information we need to
provide to GIComplexOperandMatcher.

Depends on D31418

Reviewers: aditya_nandakumar, t.p.northover, qcolombet, rovka, ab, javed.absar

Reviewed By: ab

Subscribers: dberris, kristof.beyls, igorb, llvm-commits

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

llvm-svn: 301079

Author: dsandersllvm

llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h

@@ -19,9 +19,11 @@
 #include "llvm/ADT/Optional.h"
 #include <cstdint>
 #include <bitset>
+#include <functional>
 
 namespace llvm {
 class MachineInstr;
+class MachineInstrBuilder;
 class MachineFunction;
 class MachineOperand;
 class MachineRegisterInfo;
@@ -76,6 +78,8 @@ class InstructionSelector {
   virtual bool select(MachineInstr &I) const = 0;
 
 protected:
+  typedef std::function<void(MachineInstrBuilder &)> ComplexRendererFn;
+
   InstructionSelector();
 
   /// Mutate the newly-selected instruction \p I to constrain its (possibly

llvm/include/llvm/CodeGen/MachineOperand.h

@@ -65,7 +65,6 @@ class MachineOperand {
     MO_CFIIndex,          ///< MCCFIInstruction index.
     MO_IntrinsicID,       ///< Intrinsic ID for ISel
     MO_Predicate,         ///< Generic predicate for ISel
-    MO_Placeholder,       ///< Placeholder for GlobalISel ComplexPattern result.
   };
 
 private:
@@ -768,11 +767,6 @@ class MachineOperand {
     return Op;
   }
 
-  static MachineOperand CreatePlaceholder() {
-    MachineOperand Op(MachineOperand::MO_Placeholder);
-    return Op;
-  }
-
   friend class MachineInstr;
   friend class MachineRegisterInfo;
 private:

llvm/include/llvm/Target/GlobalISel/Target.td

@@ -30,21 +30,13 @@ def s64 : LLT;
 // Definitions that inherit from this may also inherit from
 // GIComplexPatternEquiv to enable the import of SelectionDAG patterns involving
 // those ComplexPatterns.
-class GIComplexOperandMatcher<LLT type, dag operands, string matcherfn> {
+class GIComplexOperandMatcher<LLT type, string matcherfn> {
   // The expected type of the root of the match.
   //
   // TODO: We should probably support, any-type, any-scalar, and multiple types
   //       in the future.
   LLT Type = type;
 
-  // The operands that result from a successful match
-  // Should be of the form '(ops ty1, ty2, ...)' where ty1/ty2 are definitions
-  // that inherit from Operand.
-  //
-  // FIXME: Which definition is used for ty1/ty2 doesn't actually matter at the
-  //        moment. Only the number of operands is used.
-  dag Operands = operands;
-
   // The function that determines whether the operand matches. It should be of
   // the form:
   //   bool select(const MatchOperand &Root, MatchOperand &Result1)

llvm/lib/CodeGen/MIRPrinter.cpp

@@ -925,9 +925,6 @@ void MIPrinter::print(const MachineOperand &Op, const TargetRegisterInfo *TRI,
        << CmpInst::getPredicateName(Pred) << ')';
     break;
   }
-  case MachineOperand::MO_Placeholder:
-    OS << "<placeholder>";
-    break;
   }
 }
 

llvm/lib/CodeGen/MachineInstr.cpp

@@ -287,8 +287,6 @@ bool MachineOperand::isIdenticalTo(const MachineOperand &Other) const {
     return getIntrinsicID() == Other.getIntrinsicID();
   case MachineOperand::MO_Predicate:
     return getPredicate() == Other.getPredicate();
-  case MachineOperand::MO_Placeholder:
-    return true;
   }
   llvm_unreachable("Invalid machine operand type");
 }
@@ -337,8 +335,6 @@ hash_code llvm::hash_value(const MachineOperand &MO) {
     return hash_combine(MO.getType(), MO.getTargetFlags(), MO.getIntrinsicID());
   case MachineOperand::MO_Predicate:
     return hash_combine(MO.getType(), MO.getTargetFlags(), MO.getPredicate());
-  case MachineOperand::MO_Placeholder:
-    return hash_combine();
   }
   llvm_unreachable("Invalid machine operand type");
 }
@@ -515,9 +511,6 @@ void MachineOperand::print(raw_ostream &OS, ModuleSlotTracker &MST,
        << CmpInst::getPredicateName(Pred) << '>';
     break;
   }
-  case MachineOperand::MO_Placeholder:
-    OS << "<placeholder>";
-    break;
   }
   if (unsigned TF = getTargetFlags())
     OS << "[TF=" << TF << ']';

llvm/lib/Target/AArch64/AArch64InstrFormats.td

@@ -693,11 +693,11 @@ def addsub_shifted_imm32_neg : addsub_shifted_imm_neg<i32>;
 def addsub_shifted_imm64_neg : addsub_shifted_imm_neg<i64>;
 
 def gi_addsub_shifted_imm32 :
-    GIComplexOperandMatcher<s32, (ops i32imm, i32imm), "selectArithImmed">,
+    GIComplexOperandMatcher<s32, "selectArithImmed">,
     GIComplexPatternEquiv<addsub_shifted_imm32>;
 
 def gi_addsub_shifted_imm64 :
-    GIComplexOperandMatcher<s64, (ops i32imm, i32imm), "selectArithImmed">,
+    GIComplexOperandMatcher<s64, "selectArithImmed">,
     GIComplexPatternEquiv<addsub_shifted_imm64>;
 
 class neg_addsub_shifted_imm<ValueType Ty>

llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp

@@ -67,8 +67,7 @@ class AArch64InstructionSelector : public InstructionSelector {
   bool selectCompareBranch(MachineInstr &I, MachineFunction &MF,
                            MachineRegisterInfo &MRI) const;
 
-  bool selectArithImmed(MachineOperand &Root, MachineOperand &Result1,
-                        MachineOperand &Result2) const;
+  ComplexRendererFn selectArithImmed(MachineOperand &Root) const;
 
   const AArch64TargetMachine &TM;
   const AArch64Subtarget &STI;
@@ -1329,9 +1328,8 @@ bool AArch64InstructionSelector::select(MachineInstr &I) const {
 /// SelectArithImmed - Select an immediate value that can be represented as
 /// a 12-bit value shifted left by either 0 or 12.  If so, return true with
 /// Val set to the 12-bit value and Shift set to the shifter operand.
-bool AArch64InstructionSelector::selectArithImmed(
-    MachineOperand &Root, MachineOperand &Result1,
-    MachineOperand &Result2) const {
+InstructionSelector::ComplexRendererFn
+AArch64InstructionSelector::selectArithImmed(MachineOperand &Root) const {
   MachineInstr &MI = *Root.getParent();
   MachineBasicBlock &MBB = *MI.getParent();
   MachineFunction &MF = *MBB.getParent();
@@ -1350,13 +1348,13 @@ bool AArch64InstructionSelector::selectArithImmed(
   else if (Root.isReg()) {
     MachineInstr *Def = MRI.getVRegDef(Root.getReg());
     if (Def->getOpcode() != TargetOpcode::G_CONSTANT)
-      return false;
+      return nullptr;
     MachineOperand &Op1 = Def->getOperand(1);
     if (!Op1.isCImm() || Op1.getCImm()->getBitWidth() > 64)
-      return false;
+      return nullptr;
     Immed = Op1.getCImm()->getZExtValue();
   } else
-    return false;
+    return nullptr;
 
   unsigned ShiftAmt;
 
@@ -1366,14 +1364,10 @@ bool AArch64InstructionSelector::selectArithImmed(
     ShiftAmt = 12;
     Immed = Immed >> 12;
   } else
-    return false;
+    return nullptr;
 
   unsigned ShVal = AArch64_AM::getShifterImm(AArch64_AM::LSL, ShiftAmt);
-  Result1.ChangeToImmediate(Immed);
-  Result1.clearParent();
-  Result2.ChangeToImmediate(ShVal);
-  Result2.clearParent();
-  return true;
+  return [=](MachineInstrBuilder &MIB) { MIB.addImm(Immed).addImm(ShVal); };
 }
 
 namespace llvm {

llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp

@@ -661,7 +661,6 @@ static bool IsAnAddressOperand(const MachineOperand &MO) {
     return false;
   case MachineOperand::MO_IntrinsicID:
   case MachineOperand::MO_Predicate:
-  case MachineOperand::MO_Placeholder:
     llvm_unreachable("should not exist post-isel");
   }
   llvm_unreachable("unhandled machine operand type");

llvm/test/TableGen/GlobalISelEmitter.td

@@ -22,7 +22,7 @@ def complex : Operand<i32>, ComplexPattern<i32, 2, "SelectComplexPattern", []> {
   let MIOperandInfo = (ops i32imm, i32imm);
 }
 def gi_complex :
-    GIComplexOperandMatcher<s32, (ops i32imm, i32imm), "selectComplexPattern">,
+    GIComplexOperandMatcher<s32, "selectComplexPattern">,
     GIComplexPatternEquiv<complex>;
 
 def m1 : OperandWithDefaultOps <i32, (ops (i32 -1))>;
@@ -72,17 +72,15 @@ def HasB : Predicate<"Subtarget->hasB()">;
 // CHECK-NEXT:        ((/* src1 */ (MRI.getType(MI0.getOperand(1).getReg()) == (LLT::scalar(32))) &&
 // CHECK-NEXT:         ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(1).getReg(), MRI, TRI))))) &&
 // CHECK-NEXT:        ((/* src2 */ (MRI.getType(MI0.getOperand(2).getReg()) == (LLT::scalar(32))) &&
-// CHECK-NEXT:        (selectComplexPattern(MI0.getOperand(2), TempOp0, TempOp1)))) &&
+// CHECK-NEXT:        ((Renderer0 = selectComplexPattern(MI0.getOperand(2)))))) &&
 // CHECK-NEXT:        ((/* src3 */ (MRI.getType(MI0.getOperand(3).getReg()) == (LLT::scalar(32))) &&
-// CHECK-NEXT:        (selectComplexPattern(MI0.getOperand(3), TempOp2, TempOp3))))) {
+// CHECK-NEXT:        ((Renderer1 = selectComplexPattern(MI0.getOperand(3))))))) {
 // CHECK-NEXT:          // (select:i32 GPR32:i32:$src1, complex:i32:$src2, complex:i32:$src3) => (INSN2:i32 GPR32:i32:$src1, complex:i32:$src3, complex:i32:$src2)
 // CHECK-NEXT:          MachineInstrBuilder MIB = BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(MyTarget::INSN2));
 // CHECK-NEXT:          MIB.add(MI0.getOperand(0)/*dst*/);
 // CHECK-NEXT:          MIB.add(MI0.getOperand(1)/*src1*/);
-// CHECK-NEXT:          MIB.add(TempOp2);
-// CHECK-NEXT:          MIB.add(TempOp3);
-// CHECK-NEXT:          MIB.add(TempOp0);
-// CHECK-NEXT:          MIB.add(TempOp1);
+// CHECK-NEXT:          Renderer1(MIB);
+// CHECK-NEXT:          Renderer0(MIB);
 // CHECK-NEXT:          for (const auto *FromMI : {&MI0, })
 // CHECK-NEXT:            for (const auto &MMO : FromMI->memoperands())
 // CHECK-NEXT:              MIB.addMemOperand(MMO);
@@ -263,13 +261,12 @@ def MUL : I<(outs GPR32:$dst), (ins GPR32:$src2, GPR32:$src1),
 // CHECK-NEXT:        ((/* src1 */ (MRI.getType(MI0.getOperand(1).getReg()) == (LLT::scalar(32))) &&
 // CHECK-NEXT:         ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(1).getReg(), MRI, TRI))))) &&
 // CHECK-NEXT:        ((/* src2 */ (MRI.getType(MI0.getOperand(2).getReg()) == (LLT::scalar(32))) &&
-// CHECK-NEXT:        (selectComplexPattern(MI0.getOperand(2), TempOp0, TempOp1))))) {
+// CHECK-NEXT:        ((Renderer0 = selectComplexPattern(MI0.getOperand(2))))))) {
 // CHECK-NEXT:          // (sub:i32 GPR32:i32:$src1, complex:i32:$src2) => (INSN1:i32 GPR32:i32:$src1, complex:i32:$src2)
 // CHECK-NEXT:          MachineInstrBuilder MIB = BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(MyTarget::INSN1));
 // CHECK-NEXT:          MIB.add(MI0.getOperand(0)/*dst*/);
 // CHECK-NEXT:          MIB.add(MI0.getOperand(1)/*src1*/);
-// CHECK-NEXT:          MIB.add(TempOp0);
-// CHECK-NEXT:          MIB.add(TempOp1);
+// CHECK-NEXT:          Renderer0(MIB);
 // CHECK-NEXT:          for (const auto *FromMI : {&MI0, })
 // CHECK-NEXT:            for (const auto &MMO : FromMI->memoperands())
 // CHECK-NEXT:              MIB.addMemOperand(MMO);