[GlobalISel] Add `GITypeOf` special type #66079

Pierre-vh · 2023-09-12T12:38:52Z

NOTE: This is part of a stack. See #66377 for the follow-up commit which adds type inference to drop GITypeOf in most instances.

Allows creating a register/immediate that uses the same type as a matched operand.

Combined with #66077 this adds more flexibility to rewrite certain combines, e.g.

def mul_by_neg_one: GICombineRule <
  (defs root:$dst),
  (match (G_MUL $dst, $x, -1)),
  (apply (G_SUB $dst, (GITypeOf<"$x"> 0), $x))
>;

llvmbot · 2023-09-12T12:39:56Z

@llvm/pr-subscribers-backend-amdgpu

Changes

NOTE: This is part of a stack. Please only review the last commit, see #65955 to review the first commit and #66077 for the second.

Allows creating a register/immediate that uses the same type as a matched operand.

Combined with #66077 this adds more flexibility to rewrite certain combines, e.g.

def mul_by_neg_one: GICombineRule <
  (defs root:$dst),
  (match (G_MUL $dst, $x, -1)),
  (apply (G_SUB $dst, (GITypeOf<"$x"> 0), $x))
>;

`GITypeOf` is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

Patch is 140.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66079.diff

28 Files Affected:

(modified) llvm/docs/GlobalISel/MIRPatterns.rst (+44-2)
(modified) llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h (-3)
(modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h (+43-17)
(modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h (+79-46)
(modified) llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h (+10)
(modified) llvm/include/llvm/Target/GlobalISel/Combine.td (+21-4)
(modified) llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp (-12)
(modified) llvm/lib/CodeGen/GlobalISel/GIMatchTableExecutor.cpp (+4)
(modified) llvm/test/CodeGen/AArch64/GlobalISel/combine-sdiv.mir (+14-14)
(modified) llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-ashr-shl-to-sext-inreg.mir (+2-2)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fma-add-mul-pre-legalize.mir (+48-48)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fma-unmerge-values.mir (+10-10)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fold-binop-into-select.mir (+10-10)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fsub-fneg.mir (+14-14)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-imms.td (+5-7)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-patfrag-root.td (+9-15)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-permutations.td (+26-42)
(added) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-typeof.td (+49)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td (+6-8)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/operand-types.td (+27-1)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-parsing.td (+24-1)
(added) llvm/test/TableGen/GlobalISelCombinerEmitter/typeof-errors.td (+72)
(modified) llvm/test/TableGen/GlobalISelEmitter.td (+3-3)
(modified) llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp (+239-53)
(modified) llvm/utils/TableGen/GlobalISelEmitter.cpp (+4-3)
(modified) llvm/utils/TableGen/GlobalISelMatchTable.cpp (+42-3)
(modified) llvm/utils/TableGen/GlobalISelMatchTable.h (+100-8)
(modified) llvm/utils/TableGen/GlobalISelMatchTableExecutorEmitter.cpp (+1-1)

diff --git a/llvm/docs/GlobalISel/MIRPatterns.rst b/llvm/docs/GlobalISel/MIRPatterns.rst
index 51d1850a1236039..a3883b14b3e0bd6 100644
--- a/llvm/docs/GlobalISel/MIRPatterns.rst
+++ b/llvm/docs/GlobalISel/MIRPatterns.rst
@@ -101,6 +101,48 @@ pattern, you can try naming your patterns to see exactly where the issue is.
   // using $x again here copies operand 1 from G_AND into the new inst.
   (apply (COPY $root, $x))
 
+Types
+-----
+
+ValueType
+~~~~~~~~~
+
+Subclasses of ``ValueType`` are valid types, e.g. ``i32``.
+
+GITypeOf
+~~~~~~~~
+
+``GITypeOf<"$x">`` is a ``GISpecialType`` that allows for the creation of a
+register or immediate with the same type as another (register) operand.
+
+Operand:
+
+* An operand name as a string, prefixed by ``$``.
+
+Semantics:
+
+* Can only appear in an 'apply' pattern.
+* The operand name used must appear in the 'match' pattern of the
+  same ``GICombineRule``.
+
+.. code-block:: text
+  :caption: Example: Immediate
+
+  def mul_by_neg_one: GICombineRule <
+    (defs root:$root),
+    (match (G_MUL $dst, $x, -1)),
+    (apply (G_SUB $dst, (GITypeOf<"$x"> 0), $x))
+  >;
+
+.. code-block:: text
+  :caption: Example: Temp Reg
+
+  def Test0 : GICombineRule<
+    (defs root:$dst),
+    (match (G_FMUL $dst, $src, -1)),
+    (apply (G_FSUB $dst, $src, $tmp),
+           (G_FNEG GITypeOf<"$dst">:$tmp, $src))>;
+
 Builtin Operations
 ------------------
 
@@ -257,8 +299,8 @@ Common Pattern #3: Emitting a Constant Value
 When an immediate operand appears in an 'apply' pattern, the behavior
 depends on whether it's typed or not.
 
-* If the immediate is typed, a ``G_CONSTANT`` is implicitly emitted
-  (= a register operand is added to the instruction).
+* If the immediate is typed, ``MachineIRBuilder::buildConstant`` is used
+  to create a ``G_CONSTANT``. A ``G_BUILD_VECTOR`` will be used for vectors.
 * If the immediate is untyped, a simple immediate is added
   (``MachineInstrBuilder::addImm``).
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index b7c0cd4fc47faff..a896232618948a0 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -406,9 +406,6 @@ class CombinerHelper {
   void applyCombineTruncOfShift(MachineInstr &MI,
                                 std::pair &MatchInfo);
 
-  /// Transform G_MUL(x, -1) to G_SUB(0, x)
-  void applyCombineMulByNegativeOne(MachineInstr &MI);
-
   /// Return true if any explicit use operand on \p MI is defined by a
   /// G_IMPLICIT_DEF.
   bool matchAnyExplicitUseIsUndef(MachineInstr &MI);
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
index 2b0733cf9353e6c..1af12e1017c34bf 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
@@ -18,6 +18,7 @@
 #include "llvm/ADT/Bitset.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/LowLevelType.h"
 #include "llvm/CodeGen/MachineFunction.h"
@@ -40,6 +41,7 @@ class APInt;
 class APFloat;
 class GISelKnownBits;
 class MachineInstr;
+class MachineIRBuilder;
 class MachineInstrBuilder;
 class MachineFunction;
 class MachineOperand;
@@ -274,6 +276,12 @@ enum {
   /// - StoreIdx - Store location in RecordedOperands.
   GIM_RecordNamedOperand,
 
+  /// TODO DESC
+  /// - InsnID - Instruction ID
+  /// - OpIdx - Operand index
+  /// - TempTypeIdx - Temp Type Index, always negative.
+  GIM_RecordRegType,
+
   /// Fail the current try-block, or completely fail to match if there is no
   /// current try-block.
   GIM_Reject,
@@ -291,6 +299,11 @@ enum {
   /// - Opcode - The new opcode to use
   GIR_BuildMI,
 
+  /// Builds a constant and stores its result in a TempReg.
+  /// - TempRegID - Temp Register to define.
+  /// - Imm - The immediate to add
+  GIR_BuildConstant,
+
   /// Copy an operand to the specified instruction
   /// - NewInsnID - Instruction ID to modify
   /// - OldInsnID - Instruction ID to copy from
@@ -350,12 +363,6 @@ enum {
   /// - Imm - The immediate to add
   GIR_AddImm,
 
-  /// Add an CImm to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - Ty - Type of the constant immediate.
-  /// - Imm - The immediate to add
-  GIR_AddCImm,
-
   /// Render complex operands to the specified instruction
   /// - InsnID - Instruction ID to modify
   /// - RendererID - The renderer to call
@@ -501,10 +508,25 @@ class GIMatchTableExecutor {
   }
 
 protected:
+  /// Observer used by \ref executeMatchTable to record all instructions created
+  /// by the rule.
+  class GIMatchTableObserver : public GISelChangeObserver {
+  public:
+    virtual ~GIMatchTableObserver();
+
+    void erasingInstr(MachineInstr &MI) override { CreatedInsts.erase(&MI); }
+    void createdInstr(MachineInstr &MI) override { CreatedInsts.insert(&MI); }
+    void changingInstr(MachineInstr &MI) override {}
+    void changedInstr(MachineInstr &MI) override {}
+
+    // Keeps track of all instructions that have been created when applying a
+    // rule.
+    SmallDenseSet CreatedInsts;
+  };
+
   using ComplexRendererFns =
       std::optional, 4>>;
   using RecordedMIVector = SmallVector;
-  using NewMIVector = SmallVector;
 
   struct MatcherState {
     std::vector Renderers;
@@ -516,6 +538,10 @@ class GIMatchTableExecutor {
     /// list. Currently such predicates don't have more then 3 arguments.
     std::array RecordedOperands;
 
+    /// Types extracted from an instruction's operand.
+    /// Whenever a type index is negative, we look here instead.
+    SmallVector RecordedTypes;
+
     MatcherState(unsigned MaxRenderers);
   };
 
@@ -555,15 +581,15 @@ class GIMatchTableExecutor {
   /// and false otherwise.
   template 
-  bool executeMatchTable(
-      TgtExecutor &Exec, NewMIVector &OutMIs, MatcherState &State,
-      const ExecInfoTy
-          &ISelInfo,
-      const int64_t *MatchTable, const TargetInstrInfo &TII,
-      MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
-      const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
-      CodeGenCoverage *CoverageInfo,
-      GISelChangeObserver *Observer = nullptr) const;
+  bool executeMatchTable(TgtExecutor &Exec, MatcherState &State,
+                         const ExecInfoTy &ExecInfo,
+                         MachineIRBuilder &Builder, const int64_t *MatchTable,
+                         const TargetInstrInfo &TII, MachineRegisterInfo &MRI,
+                         const TargetRegisterInfo &TRI,
+                         const RegisterBankInfo &RBI,
+                         const PredicateBitset &AvailableFeatures,
+                         CodeGenCoverage *CoverageInfo) const;
 
   virtual const int64_t *getMatchTable() const {
     llvm_unreachable("Should have been overridden by tablegen if used");
@@ -592,7 +618,7 @@ class GIMatchTableExecutor {
   }
 
   virtual void runCustomAction(unsigned, const MatcherState &State,
-                               NewMIVector &OutMIs) const {
+                               ArrayRef OutMIs) const {
     llvm_unreachable("Subclass does not implement runCustomAction!");
   }
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
index 883c1ca0fe350b0..fa8bddf795a9a7f 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
@@ -18,6 +18,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h"
 #include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
+#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineOperand.h"
@@ -42,17 +43,33 @@ namespace llvm {
 template 
 bool GIMatchTableExecutor::executeMatchTable(
-    TgtExecutor &Exec, NewMIVector &OutMIs, MatcherState &State,
+    TgtExecutor &Exec, MatcherState &State,
     const ExecInfoTy
         &ExecInfo,
-    const int64_t *MatchTable, const TargetInstrInfo &TII,
-    MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
-    const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
-    CodeGenCoverage *CoverageInfo, GISelChangeObserver *Observer) const {
+    MachineIRBuilder &Builder, const int64_t *MatchTable,
+    const TargetInstrInfo &TII, MachineRegisterInfo &MRI,
+    const TargetRegisterInfo &TRI, const RegisterBankInfo &RBI,
+    const PredicateBitset &AvailableFeatures,
+    CodeGenCoverage *CoverageInfo) const {
+
+  // Setup observer
+  GIMatchTableObserver MTObserver;
+  GISelObserverWrapper Observer(&MTObserver);
+  if (auto *CurObs = Builder.getChangeObserver())
+    Observer.addObserver(CurObs);
+
+  // TODO: Set MF delegate?
+
+  // Setup builder.
+  auto RestoreOldObserver = Builder.setTemporaryChangeObserver(Observer);
 
   uint64_t CurrentIdx = 0;
   SmallVector OnFailResumeAt;
 
+  // We also record MachineInstrs manually in this vector so opcodes can address
+  // them.
+  SmallVector OutMIs;
+
   // Bypass the flag check on the instruction, and only look at the MCInstrDesc.
   bool NoFPException = !State.MIs[0]->getDesc().mayRaiseFPException();
 
@@ -71,19 +88,29 @@ bool GIMatchTableExecutor::executeMatchTable(
     return RejectAndResume;
   };
 
-  auto propagateFlags = [=](NewMIVector &OutMIs) {
-    for (auto MIB : OutMIs) {
+  auto propagateFlags = [&]() {
+    for (auto *MI : MTObserver.CreatedInsts) {
       // Set the NoFPExcept flag when no original matched instruction could
       // raise an FP exception, but the new instruction potentially might.
       uint16_t MIBFlags = Flags;
-      if (NoFPException && MIB->mayRaiseFPException())
+      if (NoFPException && MI->mayRaiseFPException())
         MIBFlags |= MachineInstr::NoFPExcept;
-      MIB.setMIFlags(MIBFlags);
+      Observer.changingInstr(*MI);
+      MI->setFlags(MIBFlags);
+      Observer.changedInstr(*MI);
     }
 
     return true;
   };
 
+  // If the index is >= 0, it's an index in the type objects generated by TableGen.
+  // If the index is <0, it's an index in the recorded types object.
+  auto getTypeFromIdx = [&](int64_t Idx) -> const LLT& {
+    if(Idx >= 0)
+      return ExecInfo.TypeObjects[Idx];
+    return State.RecordedTypes[1 - Idx];
+  };
+
   while (true) {
     assert(CurrentIdx != ~0u && "Invalid MatchTable index");
     int64_t MatcherOpcode = MatchTable[CurrentIdx++];
@@ -620,7 +647,7 @@ bool GIMatchTableExecutor::executeMatchTable(
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
       MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
       if (!MO.isReg() ||
-          MRI.getType(MO.getReg()) != ExecInfo.TypeObjects[TypeID]) {
+          MRI.getType(MO.getReg()) != getTypeFromIdx(TypeID)) {
         if (handleReject() == RejectAndGiveUp)
           return false;
       }
@@ -671,6 +698,25 @@ bool GIMatchTableExecutor::executeMatchTable(
       State.RecordedOperands[StoreIdx] = &State.MIs[InsnID]->getOperand(OpIdx);
       break;
     }
+    case GIM_RecordRegType: {
+      int64_t InsnID = MatchTable[CurrentIdx++];
+      int64_t OpIdx = MatchTable[CurrentIdx++];
+      int64_t TypeIdx = MatchTable[CurrentIdx++];
+
+      DEBUG_WITH_TYPE(TgtExecutor::getName(),
+                      dbgs() << CurrentIdx << ": GIM_RecordRegType(MIs["
+                             << InsnID << "]->getOperand(" << OpIdx
+                             << "), TypeIdx=" << TypeIdx << ")\n");
+      assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
+      assert(TypeIdx <= 0 && "Temp types always have negative indexes!");
+      // Indexes start at -1.
+      TypeIdx = 1 - TypeIdx;
+      const auto& Op = State.MIs[InsnID]->getOperand(OpIdx);
+      if(State.RecordedTypes.size() <= (uint64_t)TypeIdx)
+        State.RecordedTypes.resize(TypeIdx + 1, LLT());
+      State.RecordedTypes[TypeIdx] = MRI.getType(Op.getReg());
+      break;
+    }
     case GIM_CheckRegBankForClass: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
@@ -901,6 +947,7 @@ bool GIMatchTableExecutor::executeMatchTable(
       OutMIs[NewInsnID] = MachineInstrBuilder(*State.MIs[OldInsnID]->getMF(),
                                               State.MIs[OldInsnID]);
       OutMIs[NewInsnID]->setDesc(TII.get(NewOpcode));
+      MTObserver.CreatedInsts.insert(OutMIs[NewInsnID]);
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": GIR_MutateOpcode(OutMIs["
                              << NewInsnID << "], MIs[" << OldInsnID << "], "
@@ -914,14 +961,23 @@ bool GIMatchTableExecutor::executeMatchTable(
       if (NewInsnID >= OutMIs.size())
         OutMIs.resize(NewInsnID + 1);
 
-      OutMIs[NewInsnID] = BuildMI(*State.MIs[0]->getParent(), State.MIs[0],
-                                  MIMetadata(*State.MIs[0]), TII.get(Opcode));
+      OutMIs[NewInsnID] = Builder.buildInstr(Opcode);
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": GIR_BuildMI(OutMIs["
                              << NewInsnID << "], " << Opcode << ")\n");
       break;
     }
 
+    case GIR_BuildConstant: {
+      int64_t TempRegID = MatchTable[CurrentIdx++];
+      int64_t Imm = MatchTable[CurrentIdx++];
+      Builder.buildConstant(State.TempRegisters[TempRegID], Imm);
+      DEBUG_WITH_TYPE(TgtExecutor::getName(),
+                      dbgs() << CurrentIdx << ": GIR_BuildConstant(TempReg["
+                             << TempRegID << "], Imm=" << Imm << ")\n");
+      break;
+    }
+
     case GIR_Copy: {
       int64_t NewInsnID = MatchTable[CurrentIdx++];
       int64_t OldInsnID = MatchTable[CurrentIdx++];
@@ -1047,24 +1103,6 @@ bool GIMatchTableExecutor::executeMatchTable(
                              << "], " << Imm << ")\n");
       break;
     }
-
-    case GIR_AddCImm: {
-      int64_t InsnID = MatchTable[CurrentIdx++];
-      int64_t TypeID = MatchTable[CurrentIdx++];
-      int64_t Imm = MatchTable[CurrentIdx++];
-      assert(OutMIs[InsnID] && "Attempted to add to undefined instruction");
-
-      unsigned Width = ExecInfo.TypeObjects[TypeID].getScalarSizeInBits();
-      LLVMContext &Ctx = MF->getFunction().getContext();
-      OutMIs[InsnID].addCImm(
-          ConstantInt::get(IntegerType::get(Ctx, Width), Imm, /*signed*/ true));
-      DEBUG_WITH_TYPE(TgtExecutor::getName(),
-                      dbgs() << CurrentIdx << ": GIR_AddCImm(OutMIs[" << InsnID
-                             << "], TypeID=" << TypeID << ", Imm=" << Imm
-                             << ")\n");
-      break;
-    }
-
     case GIR_ComplexRenderer: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t RendererID = MatchTable[CurrentIdx++];
@@ -1239,8 +1277,11 @@ bool GIMatchTableExecutor::executeMatchTable(
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": GIR_EraseFromParent(MIs["
                              << InsnID << "])\n");
-      if (Observer)
-        Observer->erasingInstr(*MI);
+      // If we're erasing the insertion point, ensure we don't leave a dangling
+      // pointer in the builder.
+      if (Builder.getInsertPt() == MI)
+        Builder.setInsertPt(*MI->getParent(), ++MI->getIterator());
+      Observer.erasingInstr(*MI);
       MI->eraseFromParent();
       break;
     }
@@ -1250,7 +1291,7 @@ bool GIMatchTableExecutor::executeMatchTable(
       int64_t TypeID = MatchTable[CurrentIdx++];
 
       State.TempRegisters[TempRegID] =
-          MRI.createGenericVirtualRegister(ExecInfo.TypeObjects[TypeID]);
+          MRI.createGenericVirtualRegister(getTypeFromIdx(TypeID));
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": TempRegs[" << TempRegID
                              << "] = GIR_MakeTempReg(" << TypeID << ")\n");
@@ -1269,11 +1310,9 @@ bool GIMatchTableExecutor::executeMatchTable(
 
       Register Old = State.MIs[OldInsnID]->getOperand(OldOpIdx).getReg();
       Register New = State.MIs[NewInsnID]->getOperand(NewOpIdx).getReg();
-      if (Observer)
-        Observer->changingAllUsesOfReg(MRI, Old);
+      Observer.changingAllUsesOfReg(MRI, Old);
       MRI.replaceRegWith(Old, New);
-      if (Observer)
-        Observer->finishedChangingAllUsesOfReg();
+      Observer.finishedChangingAllUsesOfReg();
       break;
     }
     case GIR_ReplaceRegWithTempReg: {
@@ -1288,11 +1327,9 @@ bool GIMatchTableExecutor::executeMatchTable(
 
       Register Old = State.MIs[OldInsnID]->getOperand(OldOpIdx).getReg();
       Register New = State.TempRegisters[TempRegID];
-      if (Observer)
-        Observer->changingAllUsesOfReg(MRI, Old);
+      Observer.changingAllUsesOfReg(MRI, Old);
       MRI.replaceRegWith(Old, New);
-      if (Observer)
-        Observer->finishedChangingAllUsesOfReg();
+      Observer.finishedChangingAllUsesOfReg();
       break;
     }
     case GIR_Coverage: {
@@ -1309,11 +1346,7 @@ bool GIMatchTableExecutor::executeMatchTable(
     case GIR_Done:
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << ...

llvmbot · 2023-09-12T12:39:56Z

@llvm/pr-subscribers-llvm-globalisel

Changes

NOTE: This is part of a stack. Please only review the last commit, see #65955 to review the first commit and #66077 for the second.

Allows creating a register/immediate that uses the same type as a matched operand.

Combined with #66077 this adds more flexibility to rewrite certain combines, e.g.

def mul_by_neg_one: GICombineRule <
  (defs root:$dst),
  (match (G_MUL $dst, $x, -1)),
  (apply (G_SUB $dst, (GITypeOf<"$x"> 0), $x))
>;

`GITypeOf` is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

Patch is 140.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66079.diff

28 Files Affected:

(modified) llvm/docs/GlobalISel/MIRPatterns.rst (+44-2)
(modified) llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h (-3)
(modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h (+43-17)
(modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h (+79-46)
(modified) llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h (+10)
(modified) llvm/include/llvm/Target/GlobalISel/Combine.td (+21-4)
(modified) llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp (-12)
(modified) llvm/lib/CodeGen/GlobalISel/GIMatchTableExecutor.cpp (+4)
(modified) llvm/test/CodeGen/AArch64/GlobalISel/combine-sdiv.mir (+14-14)
(modified) llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-ashr-shl-to-sext-inreg.mir (+2-2)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fma-add-mul-pre-legalize.mir (+48-48)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fma-unmerge-values.mir (+10-10)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fold-binop-into-select.mir (+10-10)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fsub-fneg.mir (+14-14)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-imms.td (+5-7)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-patfrag-root.td (+9-15)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-permutations.td (+26-42)
(added) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-typeof.td (+49)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td (+6-8)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/operand-types.td (+27-1)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-parsing.td (+24-1)
(added) llvm/test/TableGen/GlobalISelCombinerEmitter/typeof-errors.td (+72)
(modified) llvm/test/TableGen/GlobalISelEmitter.td (+3-3)
(modified) llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp (+239-53)
(modified) llvm/utils/TableGen/GlobalISelEmitter.cpp (+4-3)
(modified) llvm/utils/TableGen/GlobalISelMatchTable.cpp (+42-3)
(modified) llvm/utils/TableGen/GlobalISelMatchTable.h (+100-8)
(modified) llvm/utils/TableGen/GlobalISelMatchTableExecutorEmitter.cpp (+1-1)

diff --git a/llvm/docs/GlobalISel/MIRPatterns.rst b/llvm/docs/GlobalISel/MIRPatterns.rst
index 51d1850a1236039..a3883b14b3e0bd6 100644
--- a/llvm/docs/GlobalISel/MIRPatterns.rst
+++ b/llvm/docs/GlobalISel/MIRPatterns.rst
@@ -101,6 +101,48 @@ pattern, you can try naming your patterns to see exactly where the issue is.
   // using $x again here copies operand 1 from G_AND into the new inst.
   (apply (COPY $root, $x))
 
+Types
+-----
+
+ValueType
+~~~~~~~~~
+
+Subclasses of ``ValueType`` are valid types, e.g. ``i32``.
+
+GITypeOf
+~~~~~~~~
+
+``GITypeOf<"$x">`` is a ``GISpecialType`` that allows for the creation of a
+register or immediate with the same type as another (register) operand.
+
+Operand:
+
+* An operand name as a string, prefixed by ``$``.
+
+Semantics:
+
+* Can only appear in an 'apply' pattern.
+* The operand name used must appear in the 'match' pattern of the
+  same ``GICombineRule``.
+
+.. code-block:: text
+  :caption: Example: Immediate
+
+  def mul_by_neg_one: GICombineRule <
+    (defs root:$root),
+    (match (G_MUL $dst, $x, -1)),
+    (apply (G_SUB $dst, (GITypeOf<"$x"> 0), $x))
+  >;
+
+.. code-block:: text
+  :caption: Example: Temp Reg
+
+  def Test0 : GICombineRule<
+    (defs root:$dst),
+    (match (G_FMUL $dst, $src, -1)),
+    (apply (G_FSUB $dst, $src, $tmp),
+           (G_FNEG GITypeOf<"$dst">:$tmp, $src))>;
+
 Builtin Operations
 ------------------
 
@@ -257,8 +299,8 @@ Common Pattern #3: Emitting a Constant Value
 When an immediate operand appears in an 'apply' pattern, the behavior
 depends on whether it's typed or not.
 
-* If the immediate is typed, a ``G_CONSTANT`` is implicitly emitted
-  (= a register operand is added to the instruction).
+* If the immediate is typed, ``MachineIRBuilder::buildConstant`` is used
+  to create a ``G_CONSTANT``. A ``G_BUILD_VECTOR`` will be used for vectors.
 * If the immediate is untyped, a simple immediate is added
   (``MachineInstrBuilder::addImm``).
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index b7c0cd4fc47faff..a896232618948a0 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -406,9 +406,6 @@ class CombinerHelper {
   void applyCombineTruncOfShift(MachineInstr &MI,
                                 std::pair &MatchInfo);
 
-  /// Transform G_MUL(x, -1) to G_SUB(0, x)
-  void applyCombineMulByNegativeOne(MachineInstr &MI);
-
   /// Return true if any explicit use operand on \p MI is defined by a
   /// G_IMPLICIT_DEF.
   bool matchAnyExplicitUseIsUndef(MachineInstr &MI);
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
index 2b0733cf9353e6c..1af12e1017c34bf 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
@@ -18,6 +18,7 @@
 #include "llvm/ADT/Bitset.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/LowLevelType.h"
 #include "llvm/CodeGen/MachineFunction.h"
@@ -40,6 +41,7 @@ class APInt;
 class APFloat;
 class GISelKnownBits;
 class MachineInstr;
+class MachineIRBuilder;
 class MachineInstrBuilder;
 class MachineFunction;
 class MachineOperand;
@@ -274,6 +276,12 @@ enum {
   /// - StoreIdx - Store location in RecordedOperands.
   GIM_RecordNamedOperand,
 
+  /// TODO DESC
+  /// - InsnID - Instruction ID
+  /// - OpIdx - Operand index
+  /// - TempTypeIdx - Temp Type Index, always negative.
+  GIM_RecordRegType,
+
   /// Fail the current try-block, or completely fail to match if there is no
   /// current try-block.
   GIM_Reject,
@@ -291,6 +299,11 @@ enum {
   /// - Opcode - The new opcode to use
   GIR_BuildMI,
 
+  /// Builds a constant and stores its result in a TempReg.
+  /// - TempRegID - Temp Register to define.
+  /// - Imm - The immediate to add
+  GIR_BuildConstant,
+
   /// Copy an operand to the specified instruction
   /// - NewInsnID - Instruction ID to modify
   /// - OldInsnID - Instruction ID to copy from
@@ -350,12 +363,6 @@ enum {
   /// - Imm - The immediate to add
   GIR_AddImm,
 
-  /// Add an CImm to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - Ty - Type of the constant immediate.
-  /// - Imm - The immediate to add
-  GIR_AddCImm,
-
   /// Render complex operands to the specified instruction
   /// - InsnID - Instruction ID to modify
   /// - RendererID - The renderer to call
@@ -501,10 +508,25 @@ class GIMatchTableExecutor {
   }
 
 protected:
+  /// Observer used by \ref executeMatchTable to record all instructions created
+  /// by the rule.
+  class GIMatchTableObserver : public GISelChangeObserver {
+  public:
+    virtual ~GIMatchTableObserver();
+
+    void erasingInstr(MachineInstr &MI) override { CreatedInsts.erase(&MI); }
+    void createdInstr(MachineInstr &MI) override { CreatedInsts.insert(&MI); }
+    void changingInstr(MachineInstr &MI) override {}
+    void changedInstr(MachineInstr &MI) override {}
+
+    // Keeps track of all instructions that have been created when applying a
+    // rule.
+    SmallDenseSet CreatedInsts;
+  };
+
   using ComplexRendererFns =
       std::optional, 4>>;
   using RecordedMIVector = SmallVector;
-  using NewMIVector = SmallVector;
 
   struct MatcherState {
     std::vector Renderers;
@@ -516,6 +538,10 @@ class GIMatchTableExecutor {
     /// list. Currently such predicates don't have more then 3 arguments.
     std::array RecordedOperands;
 
+    /// Types extracted from an instruction's operand.
+    /// Whenever a type index is negative, we look here instead.
+    SmallVector RecordedTypes;
+
     MatcherState(unsigned MaxRenderers);
   };
 
@@ -555,15 +581,15 @@ class GIMatchTableExecutor {
   /// and false otherwise.
   template 
-  bool executeMatchTable(
-      TgtExecutor &Exec, NewMIVector &OutMIs, MatcherState &State,
-      const ExecInfoTy
-          &ISelInfo,
-      const int64_t *MatchTable, const TargetInstrInfo &TII,
-      MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
-      const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
-      CodeGenCoverage *CoverageInfo,
-      GISelChangeObserver *Observer = nullptr) const;
+  bool executeMatchTable(TgtExecutor &Exec, MatcherState &State,
+                         const ExecInfoTy &ExecInfo,
+                         MachineIRBuilder &Builder, const int64_t *MatchTable,
+                         const TargetInstrInfo &TII, MachineRegisterInfo &MRI,
+                         const TargetRegisterInfo &TRI,
+                         const RegisterBankInfo &RBI,
+                         const PredicateBitset &AvailableFeatures,
+                         CodeGenCoverage *CoverageInfo) const;
 
   virtual const int64_t *getMatchTable() const {
     llvm_unreachable("Should have been overridden by tablegen if used");
@@ -592,7 +618,7 @@ class GIMatchTableExecutor {
   }
 
   virtual void runCustomAction(unsigned, const MatcherState &State,
-                               NewMIVector &OutMIs) const {
+                               ArrayRef OutMIs) const {
     llvm_unreachable("Subclass does not implement runCustomAction!");
   }
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
index 883c1ca0fe350b0..fa8bddf795a9a7f 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
@@ -18,6 +18,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h"
 #include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
+#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineOperand.h"
@@ -42,17 +43,33 @@ namespace llvm {
 template 
 bool GIMatchTableExecutor::executeMatchTable(
-    TgtExecutor &Exec, NewMIVector &OutMIs, MatcherState &State,
+    TgtExecutor &Exec, MatcherState &State,
     const ExecInfoTy
         &ExecInfo,
-    const int64_t *MatchTable, const TargetInstrInfo &TII,
-    MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
-    const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
-    CodeGenCoverage *CoverageInfo, GISelChangeObserver *Observer) const {
+    MachineIRBuilder &Builder, const int64_t *MatchTable,
+    const TargetInstrInfo &TII, MachineRegisterInfo &MRI,
+    const TargetRegisterInfo &TRI, const RegisterBankInfo &RBI,
+    const PredicateBitset &AvailableFeatures,
+    CodeGenCoverage *CoverageInfo) const {
+
+  // Setup observer
+  GIMatchTableObserver MTObserver;
+  GISelObserverWrapper Observer(&MTObserver);
+  if (auto *CurObs = Builder.getChangeObserver())
+    Observer.addObserver(CurObs);
+
+  // TODO: Set MF delegate?
+
+  // Setup builder.
+  auto RestoreOldObserver = Builder.setTemporaryChangeObserver(Observer);
 
   uint64_t CurrentIdx = 0;
   SmallVector OnFailResumeAt;
 
+  // We also record MachineInstrs manually in this vector so opcodes can address
+  // them.
+  SmallVector OutMIs;
+
   // Bypass the flag check on the instruction, and only look at the MCInstrDesc.
   bool NoFPException = !State.MIs[0]->getDesc().mayRaiseFPException();
 
@@ -71,19 +88,29 @@ bool GIMatchTableExecutor::executeMatchTable(
     return RejectAndResume;
   };
 
-  auto propagateFlags = [=](NewMIVector &OutMIs) {
-    for (auto MIB : OutMIs) {
+  auto propagateFlags = [&]() {
+    for (auto *MI : MTObserver.CreatedInsts) {
       // Set the NoFPExcept flag when no original matched instruction could
       // raise an FP exception, but the new instruction potentially might.
       uint16_t MIBFlags = Flags;
-      if (NoFPException && MIB->mayRaiseFPException())
+      if (NoFPException && MI->mayRaiseFPException())
         MIBFlags |= MachineInstr::NoFPExcept;
-      MIB.setMIFlags(MIBFlags);
+      Observer.changingInstr(*MI);
+      MI->setFlags(MIBFlags);
+      Observer.changedInstr(*MI);
     }
 
     return true;
   };
 
+  // If the index is >= 0, it's an index in the type objects generated by TableGen.
+  // If the index is <0, it's an index in the recorded types object.
+  auto getTypeFromIdx = [&](int64_t Idx) -> const LLT& {
+    if(Idx >= 0)
+      return ExecInfo.TypeObjects[Idx];
+    return State.RecordedTypes[1 - Idx];
+  };
+
   while (true) {
     assert(CurrentIdx != ~0u && "Invalid MatchTable index");
     int64_t MatcherOpcode = MatchTable[CurrentIdx++];
@@ -620,7 +647,7 @@ bool GIMatchTableExecutor::executeMatchTable(
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
       MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
       if (!MO.isReg() ||
-          MRI.getType(MO.getReg()) != ExecInfo.TypeObjects[TypeID]) {
+          MRI.getType(MO.getReg()) != getTypeFromIdx(TypeID)) {
         if (handleReject() == RejectAndGiveUp)
           return false;
       }
@@ -671,6 +698,25 @@ bool GIMatchTableExecutor::executeMatchTable(
       State.RecordedOperands[StoreIdx] = &State.MIs[InsnID]->getOperand(OpIdx);
       break;
     }
+    case GIM_RecordRegType: {
+      int64_t InsnID = MatchTable[CurrentIdx++];
+      int64_t OpIdx = MatchTable[CurrentIdx++];
+      int64_t TypeIdx = MatchTable[CurrentIdx++];
+
+      DEBUG_WITH_TYPE(TgtExecutor::getName(),
+                      dbgs() << CurrentIdx << ": GIM_RecordRegType(MIs["
+                             << InsnID << "]->getOperand(" << OpIdx
+                             << "), TypeIdx=" << TypeIdx << ")\n");
+      assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
+      assert(TypeIdx <= 0 && "Temp types always have negative indexes!");
+      // Indexes start at -1.
+      TypeIdx = 1 - TypeIdx;
+      const auto& Op = State.MIs[InsnID]->getOperand(OpIdx);
+      if(State.RecordedTypes.size() <= (uint64_t)TypeIdx)
+        State.RecordedTypes.resize(TypeIdx + 1, LLT());
+      State.RecordedTypes[TypeIdx] = MRI.getType(Op.getReg());
+      break;
+    }
     case GIM_CheckRegBankForClass: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
@@ -901,6 +947,7 @@ bool GIMatchTableExecutor::executeMatchTable(
       OutMIs[NewInsnID] = MachineInstrBuilder(*State.MIs[OldInsnID]->getMF(),
                                               State.MIs[OldInsnID]);
       OutMIs[NewInsnID]->setDesc(TII.get(NewOpcode));
+      MTObserver.CreatedInsts.insert(OutMIs[NewInsnID]);
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": GIR_MutateOpcode(OutMIs["
                              << NewInsnID << "], MIs[" << OldInsnID << "], "
@@ -914,14 +961,23 @@ bool GIMatchTableExecutor::executeMatchTable(
       if (NewInsnID >= OutMIs.size())
         OutMIs.resize(NewInsnID + 1);
 
-      OutMIs[NewInsnID] = BuildMI(*State.MIs[0]->getParent(), State.MIs[0],
-                                  MIMetadata(*State.MIs[0]), TII.get(Opcode));
+      OutMIs[NewInsnID] = Builder.buildInstr(Opcode);
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": GIR_BuildMI(OutMIs["
                              << NewInsnID << "], " << Opcode << ")\n");
       break;
     }
 
+    case GIR_BuildConstant: {
+      int64_t TempRegID = MatchTable[CurrentIdx++];
+      int64_t Imm = MatchTable[CurrentIdx++];
+      Builder.buildConstant(State.TempRegisters[TempRegID], Imm);
+      DEBUG_WITH_TYPE(TgtExecutor::getName(),
+                      dbgs() << CurrentIdx << ": GIR_BuildConstant(TempReg["
+                             << TempRegID << "], Imm=" << Imm << ")\n");
+      break;
+    }
+
     case GIR_Copy: {
       int64_t NewInsnID = MatchTable[CurrentIdx++];
       int64_t OldInsnID = MatchTable[CurrentIdx++];
@@ -1047,24 +1103,6 @@ bool GIMatchTableExecutor::executeMatchTable(
                              << "], " << Imm << ")\n");
       break;
     }
-
-    case GIR_AddCImm: {
-      int64_t InsnID = MatchTable[CurrentIdx++];
-      int64_t TypeID = MatchTable[CurrentIdx++];
-      int64_t Imm = MatchTable[CurrentIdx++];
-      assert(OutMIs[InsnID] && "Attempted to add to undefined instruction");
-
-      unsigned Width = ExecInfo.TypeObjects[TypeID].getScalarSizeInBits();
-      LLVMContext &Ctx = MF->getFunction().getContext();
-      OutMIs[InsnID].addCImm(
-          ConstantInt::get(IntegerType::get(Ctx, Width), Imm, /*signed*/ true));
-      DEBUG_WITH_TYPE(TgtExecutor::getName(),
-                      dbgs() << CurrentIdx << ": GIR_AddCImm(OutMIs[" << InsnID
-                             << "], TypeID=" << TypeID << ", Imm=" << Imm
-                             << ")\n");
-      break;
-    }
-
     case GIR_ComplexRenderer: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t RendererID = MatchTable[CurrentIdx++];
@@ -1239,8 +1277,11 @@ bool GIMatchTableExecutor::executeMatchTable(
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": GIR_EraseFromParent(MIs["
                              << InsnID << "])\n");
-      if (Observer)
-        Observer->erasingInstr(*MI);
+      // If we're erasing the insertion point, ensure we don't leave a dangling
+      // pointer in the builder.
+      if (Builder.getInsertPt() == MI)
+        Builder.setInsertPt(*MI->getParent(), ++MI->getIterator());
+      Observer.erasingInstr(*MI);
       MI->eraseFromParent();
       break;
     }
@@ -1250,7 +1291,7 @@ bool GIMatchTableExecutor::executeMatchTable(
       int64_t TypeID = MatchTable[CurrentIdx++];
 
       State.TempRegisters[TempRegID] =
-          MRI.createGenericVirtualRegister(ExecInfo.TypeObjects[TypeID]);
+          MRI.createGenericVirtualRegister(getTypeFromIdx(TypeID));
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": TempRegs[" << TempRegID
                              << "] = GIR_MakeTempReg(" << TypeID << ")\n");
@@ -1269,11 +1310,9 @@ bool GIMatchTableExecutor::executeMatchTable(
 
       Register Old = State.MIs[OldInsnID]->getOperand(OldOpIdx).getReg();
       Register New = State.MIs[NewInsnID]->getOperand(NewOpIdx).getReg();
-      if (Observer)
-        Observer->changingAllUsesOfReg(MRI, Old);
+      Observer.changingAllUsesOfReg(MRI, Old);
       MRI.replaceRegWith(Old, New);
-      if (Observer)
-        Observer->finishedChangingAllUsesOfReg();
+      Observer.finishedChangingAllUsesOfReg();
       break;
     }
     case GIR_ReplaceRegWithTempReg: {
@@ -1288,11 +1327,9 @@ bool GIMatchTableExecutor::executeMatchTable(
 
       Register Old = State.MIs[OldInsnID]->getOperand(OldOpIdx).getReg();
       Register New = State.TempRegisters[TempRegID];
-      if (Observer)
-        Observer->changingAllUsesOfReg(MRI, Old);
+      Observer.changingAllUsesOfReg(MRI, Old);
       MRI.replaceRegWith(Old, New);
-      if (Observer)
-        Observer->finishedChangingAllUsesOfReg();
+      Observer.finishedChangingAllUsesOfReg();
       break;
     }
     case GIR_Coverage: {
@@ -1309,11 +1346,7 @@ bool GIMatchTableExecutor::executeMatchTable(
     case GIR_Done:
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << ...

llvmbot · 2023-09-12T12:40:03Z

@llvm/pr-subscribers-backend-aarch64

Changes

NOTE: This is part of a stack. Please only review the last commit, see #65955 to review the first commit and #66077 for the second.

Allows creating a register/immediate that uses the same type as a matched operand.

Combined with #66077 this adds more flexibility to rewrite certain combines, e.g.

def mul_by_neg_one: GICombineRule <
  (defs root:$dst),
  (match (G_MUL $dst, $x, -1)),
  (apply (G_SUB $dst, (GITypeOf<"$x"> 0), $x))
>;

`GITypeOf` is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

Patch is 140.83 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66079.diff

28 Files Affected:

(modified) llvm/docs/GlobalISel/MIRPatterns.rst (+44-2)
(modified) llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h (-3)
(modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h (+43-17)
(modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h (+79-46)
(modified) llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h (+10)
(modified) llvm/include/llvm/Target/GlobalISel/Combine.td (+21-4)
(modified) llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp (-12)
(modified) llvm/lib/CodeGen/GlobalISel/GIMatchTableExecutor.cpp (+4)
(modified) llvm/test/CodeGen/AArch64/GlobalISel/combine-sdiv.mir (+14-14)
(modified) llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-ashr-shl-to-sext-inreg.mir (+2-2)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fma-add-mul-pre-legalize.mir (+48-48)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fma-unmerge-values.mir (+10-10)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fold-binop-into-select.mir (+10-10)
(modified) llvm/test/CodeGen/AMDGPU/GlobalISel/combine-fsub-fneg.mir (+14-14)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-imms.td (+5-7)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-patfrag-root.td (+9-15)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-permutations.td (+26-42)
(added) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-typeof.td (+49)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table.td (+6-8)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/operand-types.td (+27-1)
(modified) llvm/test/TableGen/GlobalISelCombinerEmitter/pattern-parsing.td (+24-1)
(added) llvm/test/TableGen/GlobalISelCombinerEmitter/typeof-errors.td (+72)
(modified) llvm/test/TableGen/GlobalISelEmitter.td (+3-3)
(modified) llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp (+239-53)
(modified) llvm/utils/TableGen/GlobalISelEmitter.cpp (+4-3)
(modified) llvm/utils/TableGen/GlobalISelMatchTable.cpp (+42-3)
(modified) llvm/utils/TableGen/GlobalISelMatchTable.h (+100-8)
(modified) llvm/utils/TableGen/GlobalISelMatchTableExecutorEmitter.cpp (+1-1)

diff --git a/llvm/docs/GlobalISel/MIRPatterns.rst b/llvm/docs/GlobalISel/MIRPatterns.rst
index 51d1850a1236039..a3883b14b3e0bd6 100644
--- a/llvm/docs/GlobalISel/MIRPatterns.rst
+++ b/llvm/docs/GlobalISel/MIRPatterns.rst
@@ -101,6 +101,48 @@ pattern, you can try naming your patterns to see exactly where the issue is.
   // using $x again here copies operand 1 from G_AND into the new inst.
   (apply (COPY $root, $x))
 
+Types
+-----
+
+ValueType
+~~~~~~~~~
+
+Subclasses of ``ValueType`` are valid types, e.g. ``i32``.
+
+GITypeOf
+~~~~~~~~
+
+``GITypeOf<"$x">`` is a ``GISpecialType`` that allows for the creation of a
+register or immediate with the same type as another (register) operand.
+
+Operand:
+
+* An operand name as a string, prefixed by ``$``.
+
+Semantics:
+
+* Can only appear in an 'apply' pattern.
+* The operand name used must appear in the 'match' pattern of the
+  same ``GICombineRule``.
+
+.. code-block:: text
+  :caption: Example: Immediate
+
+  def mul_by_neg_one: GICombineRule <
+    (defs root:$root),
+    (match (G_MUL $dst, $x, -1)),
+    (apply (G_SUB $dst, (GITypeOf<"$x"> 0), $x))
+  >;
+
+.. code-block:: text
+  :caption: Example: Temp Reg
+
+  def Test0 : GICombineRule<
+    (defs root:$dst),
+    (match (G_FMUL $dst, $src, -1)),
+    (apply (G_FSUB $dst, $src, $tmp),
+           (G_FNEG GITypeOf<"$dst">:$tmp, $src))>;
+
 Builtin Operations
 ------------------
 
@@ -257,8 +299,8 @@ Common Pattern #3: Emitting a Constant Value
 When an immediate operand appears in an 'apply' pattern, the behavior
 depends on whether it's typed or not.
 
-* If the immediate is typed, a ``G_CONSTANT`` is implicitly emitted
-  (= a register operand is added to the instruction).
+* If the immediate is typed, ``MachineIRBuilder::buildConstant`` is used
+  to create a ``G_CONSTANT``. A ``G_BUILD_VECTOR`` will be used for vectors.
 * If the immediate is untyped, a simple immediate is added
   (``MachineInstrBuilder::addImm``).
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
index b7c0cd4fc47faff..a896232618948a0 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
@@ -406,9 +406,6 @@ class CombinerHelper {
   void applyCombineTruncOfShift(MachineInstr &MI,
                                 std::pair &MatchInfo);
 
-  /// Transform G_MUL(x, -1) to G_SUB(0, x)
-  void applyCombineMulByNegativeOne(MachineInstr &MI);
-
   /// Return true if any explicit use operand on \p MI is defined by a
   /// G_IMPLICIT_DEF.
   bool matchAnyExplicitUseIsUndef(MachineInstr &MI);
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
index 2b0733cf9353e6c..1af12e1017c34bf 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
@@ -18,6 +18,7 @@
 #include "llvm/ADT/Bitset.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/LowLevelType.h"
 #include "llvm/CodeGen/MachineFunction.h"
@@ -40,6 +41,7 @@ class APInt;
 class APFloat;
 class GISelKnownBits;
 class MachineInstr;
+class MachineIRBuilder;
 class MachineInstrBuilder;
 class MachineFunction;
 class MachineOperand;
@@ -274,6 +276,12 @@ enum {
   /// - StoreIdx - Store location in RecordedOperands.
   GIM_RecordNamedOperand,
 
+  /// TODO DESC
+  /// - InsnID - Instruction ID
+  /// - OpIdx - Operand index
+  /// - TempTypeIdx - Temp Type Index, always negative.
+  GIM_RecordRegType,
+
   /// Fail the current try-block, or completely fail to match if there is no
   /// current try-block.
   GIM_Reject,
@@ -291,6 +299,11 @@ enum {
   /// - Opcode - The new opcode to use
   GIR_BuildMI,
 
+  /// Builds a constant and stores its result in a TempReg.
+  /// - TempRegID - Temp Register to define.
+  /// - Imm - The immediate to add
+  GIR_BuildConstant,
+
   /// Copy an operand to the specified instruction
   /// - NewInsnID - Instruction ID to modify
   /// - OldInsnID - Instruction ID to copy from
@@ -350,12 +363,6 @@ enum {
   /// - Imm - The immediate to add
   GIR_AddImm,
 
-  /// Add an CImm to the specified instruction
-  /// - InsnID - Instruction ID to modify
-  /// - Ty - Type of the constant immediate.
-  /// - Imm - The immediate to add
-  GIR_AddCImm,
-
   /// Render complex operands to the specified instruction
   /// - InsnID - Instruction ID to modify
   /// - RendererID - The renderer to call
@@ -501,10 +508,25 @@ class GIMatchTableExecutor {
   }
 
 protected:
+  /// Observer used by \ref executeMatchTable to record all instructions created
+  /// by the rule.
+  class GIMatchTableObserver : public GISelChangeObserver {
+  public:
+    virtual ~GIMatchTableObserver();
+
+    void erasingInstr(MachineInstr &MI) override { CreatedInsts.erase(&MI); }
+    void createdInstr(MachineInstr &MI) override { CreatedInsts.insert(&MI); }
+    void changingInstr(MachineInstr &MI) override {}
+    void changedInstr(MachineInstr &MI) override {}
+
+    // Keeps track of all instructions that have been created when applying a
+    // rule.
+    SmallDenseSet CreatedInsts;
+  };
+
   using ComplexRendererFns =
       std::optional, 4>>;
   using RecordedMIVector = SmallVector;
-  using NewMIVector = SmallVector;
 
   struct MatcherState {
     std::vector Renderers;
@@ -516,6 +538,10 @@ class GIMatchTableExecutor {
     /// list. Currently such predicates don't have more then 3 arguments.
     std::array RecordedOperands;
 
+    /// Types extracted from an instruction's operand.
+    /// Whenever a type index is negative, we look here instead.
+    SmallVector RecordedTypes;
+
     MatcherState(unsigned MaxRenderers);
   };
 
@@ -555,15 +581,15 @@ class GIMatchTableExecutor {
   /// and false otherwise.
   template 
-  bool executeMatchTable(
-      TgtExecutor &Exec, NewMIVector &OutMIs, MatcherState &State,
-      const ExecInfoTy
-          &ISelInfo,
-      const int64_t *MatchTable, const TargetInstrInfo &TII,
-      MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
-      const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
-      CodeGenCoverage *CoverageInfo,
-      GISelChangeObserver *Observer = nullptr) const;
+  bool executeMatchTable(TgtExecutor &Exec, MatcherState &State,
+                         const ExecInfoTy &ExecInfo,
+                         MachineIRBuilder &Builder, const int64_t *MatchTable,
+                         const TargetInstrInfo &TII, MachineRegisterInfo &MRI,
+                         const TargetRegisterInfo &TRI,
+                         const RegisterBankInfo &RBI,
+                         const PredicateBitset &AvailableFeatures,
+                         CodeGenCoverage *CoverageInfo) const;
 
   virtual const int64_t *getMatchTable() const {
     llvm_unreachable("Should have been overridden by tablegen if used");
@@ -592,7 +618,7 @@ class GIMatchTableExecutor {
   }
 
   virtual void runCustomAction(unsigned, const MatcherState &State,
-                               NewMIVector &OutMIs) const {
+                               ArrayRef OutMIs) const {
     llvm_unreachable("Subclass does not implement runCustomAction!");
   }
 
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
index 883c1ca0fe350b0..fa8bddf795a9a7f 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
@@ -18,6 +18,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h"
 #include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
+#include "llvm/CodeGen/GlobalISel/MachineIRBuilder.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineOperand.h"
@@ -42,17 +43,33 @@ namespace llvm {
 template 
 bool GIMatchTableExecutor::executeMatchTable(
-    TgtExecutor &Exec, NewMIVector &OutMIs, MatcherState &State,
+    TgtExecutor &Exec, MatcherState &State,
     const ExecInfoTy
         &ExecInfo,
-    const int64_t *MatchTable, const TargetInstrInfo &TII,
-    MachineRegisterInfo &MRI, const TargetRegisterInfo &TRI,
-    const RegisterBankInfo &RBI, const PredicateBitset &AvailableFeatures,
-    CodeGenCoverage *CoverageInfo, GISelChangeObserver *Observer) const {
+    MachineIRBuilder &Builder, const int64_t *MatchTable,
+    const TargetInstrInfo &TII, MachineRegisterInfo &MRI,
+    const TargetRegisterInfo &TRI, const RegisterBankInfo &RBI,
+    const PredicateBitset &AvailableFeatures,
+    CodeGenCoverage *CoverageInfo) const {
+
+  // Setup observer
+  GIMatchTableObserver MTObserver;
+  GISelObserverWrapper Observer(&MTObserver);
+  if (auto *CurObs = Builder.getChangeObserver())
+    Observer.addObserver(CurObs);
+
+  // TODO: Set MF delegate?
+
+  // Setup builder.
+  auto RestoreOldObserver = Builder.setTemporaryChangeObserver(Observer);
 
   uint64_t CurrentIdx = 0;
   SmallVector OnFailResumeAt;
 
+  // We also record MachineInstrs manually in this vector so opcodes can address
+  // them.
+  SmallVector OutMIs;
+
   // Bypass the flag check on the instruction, and only look at the MCInstrDesc.
   bool NoFPException = !State.MIs[0]->getDesc().mayRaiseFPException();
 
@@ -71,19 +88,29 @@ bool GIMatchTableExecutor::executeMatchTable(
     return RejectAndResume;
   };
 
-  auto propagateFlags = [=](NewMIVector &OutMIs) {
-    for (auto MIB : OutMIs) {
+  auto propagateFlags = [&]() {
+    for (auto *MI : MTObserver.CreatedInsts) {
       // Set the NoFPExcept flag when no original matched instruction could
       // raise an FP exception, but the new instruction potentially might.
       uint16_t MIBFlags = Flags;
-      if (NoFPException && MIB->mayRaiseFPException())
+      if (NoFPException && MI->mayRaiseFPException())
         MIBFlags |= MachineInstr::NoFPExcept;
-      MIB.setMIFlags(MIBFlags);
+      Observer.changingInstr(*MI);
+      MI->setFlags(MIBFlags);
+      Observer.changedInstr(*MI);
     }
 
     return true;
   };
 
+  // If the index is >= 0, it's an index in the type objects generated by TableGen.
+  // If the index is <0, it's an index in the recorded types object.
+  auto getTypeFromIdx = [&](int64_t Idx) -> const LLT& {
+    if(Idx >= 0)
+      return ExecInfo.TypeObjects[Idx];
+    return State.RecordedTypes[1 - Idx];
+  };
+
   while (true) {
     assert(CurrentIdx != ~0u && "Invalid MatchTable index");
     int64_t MatcherOpcode = MatchTable[CurrentIdx++];
@@ -620,7 +647,7 @@ bool GIMatchTableExecutor::executeMatchTable(
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
       MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
       if (!MO.isReg() ||
-          MRI.getType(MO.getReg()) != ExecInfo.TypeObjects[TypeID]) {
+          MRI.getType(MO.getReg()) != getTypeFromIdx(TypeID)) {
         if (handleReject() == RejectAndGiveUp)
           return false;
       }
@@ -671,6 +698,25 @@ bool GIMatchTableExecutor::executeMatchTable(
       State.RecordedOperands[StoreIdx] = &State.MIs[InsnID]->getOperand(OpIdx);
       break;
     }
+    case GIM_RecordRegType: {
+      int64_t InsnID = MatchTable[CurrentIdx++];
+      int64_t OpIdx = MatchTable[CurrentIdx++];
+      int64_t TypeIdx = MatchTable[CurrentIdx++];
+
+      DEBUG_WITH_TYPE(TgtExecutor::getName(),
+                      dbgs() << CurrentIdx << ": GIM_RecordRegType(MIs["
+                             << InsnID << "]->getOperand(" << OpIdx
+                             << "), TypeIdx=" << TypeIdx << ")\n");
+      assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
+      assert(TypeIdx <= 0 && "Temp types always have negative indexes!");
+      // Indexes start at -1.
+      TypeIdx = 1 - TypeIdx;
+      const auto& Op = State.MIs[InsnID]->getOperand(OpIdx);
+      if(State.RecordedTypes.size() <= (uint64_t)TypeIdx)
+        State.RecordedTypes.resize(TypeIdx + 1, LLT());
+      State.RecordedTypes[TypeIdx] = MRI.getType(Op.getReg());
+      break;
+    }
     case GIM_CheckRegBankForClass: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
@@ -901,6 +947,7 @@ bool GIMatchTableExecutor::executeMatchTable(
       OutMIs[NewInsnID] = MachineInstrBuilder(*State.MIs[OldInsnID]->getMF(),
                                               State.MIs[OldInsnID]);
       OutMIs[NewInsnID]->setDesc(TII.get(NewOpcode));
+      MTObserver.CreatedInsts.insert(OutMIs[NewInsnID]);
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": GIR_MutateOpcode(OutMIs["
                              << NewInsnID << "], MIs[" << OldInsnID << "], "
@@ -914,14 +961,23 @@ bool GIMatchTableExecutor::executeMatchTable(
       if (NewInsnID >= OutMIs.size())
         OutMIs.resize(NewInsnID + 1);
 
-      OutMIs[NewInsnID] = BuildMI(*State.MIs[0]->getParent(), State.MIs[0],
-                                  MIMetadata(*State.MIs[0]), TII.get(Opcode));
+      OutMIs[NewInsnID] = Builder.buildInstr(Opcode);
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": GIR_BuildMI(OutMIs["
                              << NewInsnID << "], " << Opcode << ")\n");
       break;
     }
 
+    case GIR_BuildConstant: {
+      int64_t TempRegID = MatchTable[CurrentIdx++];
+      int64_t Imm = MatchTable[CurrentIdx++];
+      Builder.buildConstant(State.TempRegisters[TempRegID], Imm);
+      DEBUG_WITH_TYPE(TgtExecutor::getName(),
+                      dbgs() << CurrentIdx << ": GIR_BuildConstant(TempReg["
+                             << TempRegID << "], Imm=" << Imm << ")\n");
+      break;
+    }
+
     case GIR_Copy: {
       int64_t NewInsnID = MatchTable[CurrentIdx++];
       int64_t OldInsnID = MatchTable[CurrentIdx++];
@@ -1047,24 +1103,6 @@ bool GIMatchTableExecutor::executeMatchTable(
                              << "], " << Imm << ")\n");
       break;
     }
-
-    case GIR_AddCImm: {
-      int64_t InsnID = MatchTable[CurrentIdx++];
-      int64_t TypeID = MatchTable[CurrentIdx++];
-      int64_t Imm = MatchTable[CurrentIdx++];
-      assert(OutMIs[InsnID] && "Attempted to add to undefined instruction");
-
-      unsigned Width = ExecInfo.TypeObjects[TypeID].getScalarSizeInBits();
-      LLVMContext &Ctx = MF->getFunction().getContext();
-      OutMIs[InsnID].addCImm(
-          ConstantInt::get(IntegerType::get(Ctx, Width), Imm, /*signed*/ true));
-      DEBUG_WITH_TYPE(TgtExecutor::getName(),
-                      dbgs() << CurrentIdx << ": GIR_AddCImm(OutMIs[" << InsnID
-                             << "], TypeID=" << TypeID << ", Imm=" << Imm
-                             << ")\n");
-      break;
-    }
-
     case GIR_ComplexRenderer: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t RendererID = MatchTable[CurrentIdx++];
@@ -1239,8 +1277,11 @@ bool GIMatchTableExecutor::executeMatchTable(
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": GIR_EraseFromParent(MIs["
                              << InsnID << "])\n");
-      if (Observer)
-        Observer->erasingInstr(*MI);
+      // If we're erasing the insertion point, ensure we don't leave a dangling
+      // pointer in the builder.
+      if (Builder.getInsertPt() == MI)
+        Builder.setInsertPt(*MI->getParent(), ++MI->getIterator());
+      Observer.erasingInstr(*MI);
       MI->eraseFromParent();
       break;
     }
@@ -1250,7 +1291,7 @@ bool GIMatchTableExecutor::executeMatchTable(
       int64_t TypeID = MatchTable[CurrentIdx++];
 
       State.TempRegisters[TempRegID] =
-          MRI.createGenericVirtualRegister(ExecInfo.TypeObjects[TypeID]);
+          MRI.createGenericVirtualRegister(getTypeFromIdx(TypeID));
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << CurrentIdx << ": TempRegs[" << TempRegID
                              << "] = GIR_MakeTempReg(" << TypeID << ")\n");
@@ -1269,11 +1310,9 @@ bool GIMatchTableExecutor::executeMatchTable(
 
       Register Old = State.MIs[OldInsnID]->getOperand(OldOpIdx).getReg();
       Register New = State.MIs[NewInsnID]->getOperand(NewOpIdx).getReg();
-      if (Observer)
-        Observer->changingAllUsesOfReg(MRI, Old);
+      Observer.changingAllUsesOfReg(MRI, Old);
       MRI.replaceRegWith(Old, New);
-      if (Observer)
-        Observer->finishedChangingAllUsesOfReg();
+      Observer.finishedChangingAllUsesOfReg();
       break;
     }
     case GIR_ReplaceRegWithTempReg: {
@@ -1288,11 +1327,9 @@ bool GIMatchTableExecutor::executeMatchTable(
 
       Register Old = State.MIs[OldInsnID]->getOperand(OldOpIdx).getReg();
       Register New = State.TempRegisters[TempRegID];
-      if (Observer)
-        Observer->changingAllUsesOfReg(MRI, Old);
+      Observer.changingAllUsesOfReg(MRI, Old);
       MRI.replaceRegWith(Old, New);
-      if (Observer)
-        Observer->finishedChangingAllUsesOfReg();
+      Observer.finishedChangingAllUsesOfReg();
       break;
     }
     case GIR_Coverage: {
@@ -1309,11 +1346,7 @@ bool GIMatchTableExecutor::executeMatchTable(
     case GIR_Done:
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
                       dbgs() << ...

llvm/docs/GlobalISel/MIRPatterns.rst

llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h

llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h

Allows creating a register/immediate that uses the same type as a matched operand.

Pierre-vh · 2023-10-24T10:09:30Z

Ping

arsenm

lgtm with nit

llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h

Pierre-vh · 2023-10-31T06:47:56Z

@arsenm can it land ? (if so, please approve) thanks

Pierre-vh requested review from a team as code owners September 12, 2023 12:38

llvmbot added backend:AArch64 backend:AMDGPU llvm:globalisel labels Sep 12, 2023

arsenm reviewed Sep 12, 2023

View reviewed changes

llvm/docs/GlobalISel/MIRPatterns.rst Show resolved Hide resolved

llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h Outdated Show resolved Hide resolved

Pierre-vh force-pushed the gi-typeof branch from fa6632a to 1f4c714 Compare September 12, 2023 13:17

Pierre-vh mentioned this pull request Sep 14, 2023

[TableGen][GlobalISel] Add rule-wide type inference #66377

Merged

Pierre-vh mentioned this pull request Oct 17, 2023

[TableGen] Use buildConstant to emit apply pattern immediates #66077

Merged

Pierre-vh force-pushed the gi-typeof branch from 1f4c714 to 01665ae Compare October 17, 2023 08:48

Pierre-vh requested review from aemerson and jayfoad and removed request for a team October 18, 2023 05:48

Pierre-vh force-pushed the gi-typeof branch from 01665ae to cf3ba9d Compare October 18, 2023 05:49

aemerson reviewed Oct 19, 2023

View reviewed changes

llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h Outdated Show resolved Hide resolved

[GlobalISel] Add GITypeOf special type

27f5e90

Allows creating a register/immediate that uses the same type as a matched operand.

Pierre-vh force-pushed the gi-typeof branch from cf3ba9d to 27f5e90 Compare October 19, 2023 05:49

Pierre-vh added 2 commits October 19, 2023 08:00

Restore AddCImm

e803b89

Fix comments

653f197

Pierre-vh requested review from arsenm and aemerson October 26, 2023 06:05

arsenm reviewed Oct 26, 2023

View reviewed changes

llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h Outdated Show resolved Hide resolved

Return LLT by value

88f1ba9

Pierre-vh requested a review from arsenm October 26, 2023 08:08

aemerson approved these changes Oct 31, 2023

View reviewed changes

Pierre-vh merged commit b26e6a8 into llvm:main Oct 31, 2023
3 checks passed

Pierre-vh deleted the gi-typeof branch October 31, 2023 08:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GlobalISel] Add `GITypeOf` special type #66079

[GlobalISel] Add `GITypeOf` special type #66079

Pierre-vh commented Sep 12, 2023 •

edited

Loading

llvmbot commented Sep 12, 2023

`GITypeOf` is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

llvmbot commented Sep 12, 2023

`GITypeOf` is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

llvmbot commented Sep 12, 2023

`GITypeOf` is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

Pierre-vh commented Oct 24, 2023

arsenm left a comment

Pierre-vh commented Oct 31, 2023

[GlobalISel] Add GITypeOf special type #66079

[GlobalISel] Add GITypeOf special type #66079

Conversation

Pierre-vh commented Sep 12, 2023 • edited Loading

llvmbot commented Sep 12, 2023

GITypeOf is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

llvmbot commented Sep 12, 2023

GITypeOf is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

llvmbot commented Sep 12, 2023

GITypeOf is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

Pierre-vh commented Oct 24, 2023

arsenm left a comment

Choose a reason for hiding this comment

Pierre-vh commented Oct 31, 2023

[GlobalISel] Add `GITypeOf` special type #66079

[GlobalISel] Add `GITypeOf` special type #66079

Pierre-vh commented Sep 12, 2023 •

edited

Loading

`GITypeOf` is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

`GITypeOf` is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).

`GITypeOf` is verbose but needed. Inference isn't good enough in MIR patterns yet (for these things it's actually non-existent even).