[GlobalISel] Enhance iPTR type support in SDAG patterns #111503

e-kud · 2024-10-08T08:55:52Z

This adds a check for integer types covered by iPTR type. It is dictated by the fact that insertelt node has SDTCisPtrTy constraint on the index operand. At the same time, this constraint can't be replaced by SDTCisInt as it doesn't reduce the amount of possible types to the only one possible.

This adds checks for integer types covered by iPTR type. It is dictated by the fact that insertelt node has SDTCisPtrTy constraint on the immediate operand. At the same time, this constraint can't be replaced by SDTCisInt, as it doesn't reduce the amount of possible types to the only one possible.

llvmbot · 2024-10-08T08:56:33Z

@llvm/pr-subscribers-tablegen

@llvm/pr-subscribers-llvm-globalisel

Author: Evgenii Kudriashov (e-kud)

Changes

This adds checks for integer types covered by iPTR type. It is dictated by the fact that insertelt node has SDTCisPtrTy constraint on the index operand. At the same time, this constraint can't be replaced by SDTCisInt as it doesn't reduce the amount of possible types to the only one possible.

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

6 Files Affected:

(modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h (+1)
(modified) llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h (+8-4)
(modified) llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td (+1)
(modified) llvm/test/TableGen/GlobalISelEmitter.td (+92-51)
(modified) llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp (+15-2)
(modified) llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h (+23)

diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
index 7b42722ca8d4f1..95c8574ff27079 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
@@ -237,6 +237,7 @@ enum {
   /// - OpIdx(ULEB128) - Operand index
   /// - SizeInBits(ULEB128) - The size of the pointer value in bits.
   GIM_CheckPointerToAny,
+  GIM_CheckIntPtr,
 
   /// Check the register bank for the specified operand
   /// - InsnID(ULEB128) - Instruction ID
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
index 9f8eb030a96c64..648e9394276470 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
@@ -719,14 +719,17 @@ bool GIMatchTableExecutor::executeMatchTable(
       }
       break;
     }
+    case GIM_CheckIntPtr:
     case GIM_CheckPointerToAny: {
+      bool IsPointerCheck = MatcherOpcode == GIM_CheckPointerToAny;
       uint64_t InsnID = readULEB();
       uint64_t OpIdx = readULEB();
       uint64_t SizeInBits = readULEB();
 
       DEBUG_WITH_TYPE(TgtExecutor::getName(),
-                      dbgs() << CurrentIdx << ": GIM_CheckPointerToAny(MIs["
-                             << InsnID << "]->getOperand(" << OpIdx
+                      dbgs() << CurrentIdx << ": GIM_Check"
+                             << (IsPointerCheck ? "PointerToAny" : "IntPtr")
+                             << "(MIs[" << InsnID << "]->getOperand(" << OpIdx
                              << "), SizeInBits=" << SizeInBits << ")\n");
       assert(State.MIs[InsnID] != nullptr && "Used insn before defined");
       MachineOperand &MO = State.MIs[InsnID]->getOperand(OpIdx);
@@ -735,14 +738,15 @@ bool GIMatchTableExecutor::executeMatchTable(
       // iPTR must be looked up in the target.
       if (SizeInBits == 0) {
         MachineFunction *MF = State.MIs[InsnID]->getParent()->getParent();
-        const unsigned AddrSpace = Ty.getAddressSpace();
+        const unsigned AddrSpace = IsPointerCheck ? Ty.getAddressSpace() : 0;
         SizeInBits = MF->getDataLayout().getPointerSizeInBits(AddrSpace);
       }
 
       assert(SizeInBits != 0 && "Pointer size must be known");
 
       if (MO.isReg()) {
-        if (!Ty.isPointer() || Ty.getSizeInBits() != SizeInBits)
+        if ((IsPointerCheck && !Ty.isPointer()) ||
+            Ty.getSizeInBits() != SizeInBits)
           if (handleReject() == RejectAndGiveUp)
             return false;
       } else if (handleReject() == RejectAndGiveUp)
diff --git a/llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td b/llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td
index d9121cf166e5aa..33089de2457952 100644
--- a/llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td
+++ b/llvm/include/llvm/Target/GlobalISel/SelectionDAGCompat.td
@@ -145,6 +145,7 @@ def : GINodeEquiv<G_CTTZ_ZERO_UNDEF, cttz_zero_undef>;
 def : GINodeEquiv<G_CTPOP, ctpop>;
 def : GINodeEquiv<G_EXTRACT_VECTOR_ELT, extractelt>;
 def : GINodeEquiv<G_INSERT_VECTOR_ELT, vector_insert>;
+def : GINodeEquiv<G_INSERT_VECTOR_ELT, insertelt>;
 def : GINodeEquiv<G_CONCAT_VECTORS, concat_vectors>;
 def : GINodeEquiv<G_BUILD_VECTOR, build_vector>;
 def : GINodeEquiv<G_FCEIL, fceil>;
diff --git a/llvm/test/TableGen/GlobalISelEmitter.td b/llvm/test/TableGen/GlobalISelEmitter.td
index 7dbaf4390c0f70..13d46d7e792ab2 100644
--- a/llvm/test/TableGen/GlobalISelEmitter.td
+++ b/llvm/test/TableGen/GlobalISelEmitter.td
@@ -95,11 +95,13 @@ def HasC : Predicate<"Subtarget->hasC()"> { let RecomputePerFunction = 1; }
 // CHECK-NEXT:  enum {
 // CHECK-NEXT:    GILLT_p0s32
 // CHECK-NEXT:    GILLT_s32,
+// CHECK-NEXT:    GILLT_v4s32,
 // CHECK-NEXT:  }
-// CHECK-NEXT:  const static size_t NumTypeObjects = 2;
+// CHECK-NEXT:  const static size_t NumTypeObjects = 3;
 // CHECK-NEXT:  const static LLT TypeObjects[] = {
 // CHECK-NEXT:    LLT::pointer(0, 32),
 // CHECK-NEXT:    LLT::scalar(32),
+// CHECK-NEXT:    LLT::vector(ElementCount::getFixed(4), 32),
 // CHECK-NEXT:  };
 
 // CHECK-LABEL: enum SubtargetFeatureBits : uint8_t {
@@ -242,7 +244,7 @@ def HasC : Predicate<"Subtarget->hasC()"> { let RecomputePerFunction = 1; }
 // R19O-NEXT:    GIM_RootCheckType, /*Op*/2, /*Type*/GILLT_s32,
 // R19O-NEXT:    GIM_RootCheckType, /*Op*/3, /*Type*/GILLT_s32,
 //
-// R19C-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
+// R19C-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
 //
 // R19O-NEXT:    GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
 // R19O-NEXT:    GIM_RootCheckRegBankForClass, /*Op*/1, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
@@ -297,7 +299,7 @@ def HasC : Predicate<"Subtarget->hasC()"> { let RecomputePerFunction = 1; }
 // R19C-NEXT:    GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/GIMT_Encode2(0), /*SubOperand*/0, // src2a
 // R19C-NEXT:    GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
 // R19C-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// R19C-NEXT:    // GIR_Coverage, 20,
+// R19C-NEXT:    // GIR_Coverage, [[ID]],
 // R19C-NEXT:    GIR_EraseRootFromParent_Done,
 // R19C-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 //
@@ -330,12 +332,12 @@ def : Pat<(select GPR32:$src1, (complex_rr GPR32:$src2a, GPR32:$src2b),
 // R21O-NEXT:    GIM_RootCheckType, /*Op*/2, /*Type*/GILLT_s32,
 // R21O-NEXT:    GIM_RootCheckType, /*Op*/3, /*Type*/GILLT_s32,
 //
-// R21C-NEXT:  GIM_Try, /*On fail goto*//*Label [[PREV_NUM:[0-9]+]]*/ GIMT_Encode4([[PREV:[0-9]+]]), // Rule ID 20 //
+// R21C-NEXT:  GIM_Try, /*On fail goto*//*Label [[PREV_NUM:[0-9]+]]*/ GIMT_Encode4([[PREV:[0-9]+]]), // Rule ID 21 //
 // R21C-NOT:     GIR_EraseRootFromParent_Done,
-// R21C:         // GIR_Coverage, 20,
+// R21C:         // GIR_Coverage, 21,
 // R21C-NEXT:    GIR_EraseRootFromParent_Done,
 // R21C-NEXT:  // Label [[PREV_NUM]]: @[[PREV]]
-// R21C-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID 22 //
+// R21C-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID 23 //
 //
 // R21O-NEXT:    GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
 // R21O-NEXT:    GIM_RootCheckRegBankForClass, /*Op*/1, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
@@ -365,7 +367,7 @@ def : Pat<(select GPR32:$src1, (complex_rr GPR32:$src2a, GPR32:$src2b),
 // R21C-NEXT:    GIR_ComplexRenderer, /*InsnID*/0, /*RendererID*/GIMT_Encode2(0),
 // R21C-NEXT:    GIR_MergeMemOperands, /*InsnID*/0, /*NumInsns*/1, /*MergeInsnID's*/0
 // R21C-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// R21C-NEXT:    // GIR_Coverage, 22,
+// R21C-NEXT:    // GIR_Coverage, 23,
 // R21C-NEXT:    GIR_EraseRootFromParent_Done,
 // R21C-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 //
@@ -389,10 +391,10 @@ def : Pat<(select GPR32:$src1, (complex_rr GPR32:$src2a, GPR32:$src2b),
 // R20O-NEXT:    GIM_RootCheckType, /*Op*/2, /*Type*/GILLT_s32,
 // R20O-NEXT:    GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
 //
-// R20N:       GIM_Try, /*On fail goto*//*Label [[PREV_NUM:[0-9]+]]*/ GIMT_Encode4([[PREV:[0-9]+]]), // Rule ID 22 //
+// R20N:       GIM_Try, /*On fail goto*//*Label [[PREV_NUM:[0-9]+]]*/ GIMT_Encode4([[PREV:[0-9]+]]), // Rule ID 23 //
 // R20N:       // Label [[PREV_NUM]]: @[[PREV]]
 //
-// R20C-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID 21 //
+// R20C-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID 22 //
 //
 // R20N-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
 // R20N-NEXT:    GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_SUB),
@@ -413,7 +415,7 @@ def : Pat<(select GPR32:$src1, (complex_rr GPR32:$src2a, GPR32:$src2b),
 // R20C-NEXT:    GIR_RootToRootCopy, /*OpIdx*/1, // src1
 // R20C-NEXT:    GIR_ComplexRenderer, /*InsnID*/0, /*RendererID*/GIMT_Encode2(0),
 // R20C-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// R20C-NEXT:    // GIR_Coverage, 21,
+// R20C-NEXT:    // GIR_Coverage, 22,
 // R20C-NEXT:    GIR_EraseRootFromParent_Done,
 // R20C-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 //
@@ -453,7 +455,7 @@ def : Pat<(frag GPR32:$src1, complex:$src2, complex:$src3),
 // R00O-NEXT:    GIM_RootCheckType, /*Op*/2, /*Type*/GILLT_s32,
 // R00O-NEXT:    GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
 //
-// R00C:       GIM_Try, /*On fail goto*//*Label [[PREV_NUM:[0-9]+]]*/ GIMT_Encode4([[PREV:[0-9]+]]), // Rule ID 21 //
+// R00C:       GIM_Try, /*On fail goto*//*Label [[PREV_NUM:[0-9]+]]*/ GIMT_Encode4([[PREV:[0-9]+]]), // Rule ID 22 //
 // R00C:       // Label [[PREV_NUM]]: @[[PREV]]
 //
 // R00C-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID 0 //
@@ -513,7 +515,7 @@ def : Pat<(frag GPR32:$src1, complex:$src2, complex:$src3),
 // R00O-NEXT:  GIM_Reject,
 // R00O:       // Label [[DEFAULT_NUM]]: @[[DEFAULT]]
 // R00O-NEXT:  GIM_Reject,
-// R00O-NEXT:  }; // Size: 1832 bytes
+// R00O-NEXT:  }; // Size: 1907 bytes
 
 def INSNBOB : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2, GPR32:$src3, GPR32:$src4),
                  [(set GPR32:$dst,
@@ -733,7 +735,7 @@ def XORIb : I<(outs GPR32:$dst), (ins mb:$src2, GPR32:$src1),
 // This must precede the 3-register variants because constant immediates have
 // priority over register banks.
 //
-// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
+// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
 // NOOPT-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
 // NOOPT-NEXT:    GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_XOR),
 // NOOPT-NEXT:    // MIs[0] DstI[dst]
@@ -751,16 +753,55 @@ def XORIb : I<(outs GPR32:$dst), (ins mb:$src2, GPR32:$src1),
 // NOOPT-NEXT:    GIR_AddRegister, /*InsnID*/0, GIMT_Encode2(MyTarget::R0),
 // NOOPT-NEXT:    GIR_RootToRootCopy, /*OpIdx*/1, // Wm
 // NOOPT-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// NOOPT-NEXT:    // GIR_Coverage, 23,
+// NOOPT-NEXT:    // GIR_Coverage, [[ID]],
 // NOOPT-NEXT:    GIR_EraseRootFromParent_Done,
 // NOOPT-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 
 def ORN : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2), []>;
 def : Pat<(not GPR32:$Wm), (ORN R0, GPR32:$Wm)>;
 
+//===- Test a pattern with iPTR type of operand. --------------------------===//
+//
+// NOOPT-NEXT:    GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
+// NOOPT-NEXT:      GIM_CheckNumOperands, /*MI*/0, /*Expected*/4,
+// NOOPT-NEXT:      GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_INSERT_VECTOR_ELT),
+// NOOPT-NEXT:      // MIs[0] DstI[dst]
+// NOOPT-NEXT:      GIM_RootCheckType, /*Op*/0, /*Type*/GILLT_v4s32,
+// NOOPT-NEXT:      GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::VecReg128RegClassID),
+// NOOPT-NEXT:      // MIs[0] src1
+// NOOPT-NEXT:      GIM_RootCheckType, /*Op*/1, /*Type*/GILLT_v4s32,
+// NOOPT-NEXT:      GIM_RootCheckRegBankForClass, /*Op*/1, /*RC*/GIMT_Encode2(MyTarget::VecReg128RegClassID),
+// NOOPT-NEXT:      // MIs[0] src2
+// NOOPT-NEXT:      GIM_RootCheckType, /*Op*/2, /*Type*/GILLT_s32,
+// NOOPT-NEXT:      GIM_RootCheckRegBankForClass, /*Op*/2, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
+// NOOPT-NEXT:      // MIs[0] src3
+// NOOPT-NEXT:      GIM_CheckIntPtr, /*MI*/0, /*Op*/3, /*SizeInBits*/0,
+// NOOPT-NEXT:      GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/3, // MIs[1]
+// NOOPT-NEXT:      GIM_CheckNumOperands, /*MI*/1, /*Expected*/2,
+// NOOPT-NEXT:      GIM_CheckOpcode, /*MI*/1, GIMT_Encode2(TargetOpcode::G_CONSTANT),
+// NOOPT-NEXT:      // MIs[1] Operand 0
+// NOOPT-NEXT:      GIM_CheckIntPtr, /*MI*/1, /*Op*/0, /*SizeInBits*/0,
+// NOOPT-NEXT:      // MIs[1] Operand 1
+// NOOPT-NEXT:      // No operand predicates
+// NOOPT-NEXT:      GIM_CheckIsSafeToFold, /*NumInsns*/1,
+// NOOPT-NEXT:      // (insertelt:{ *:[v4i32] } VecReg128:{ *:[v4i32] }:$src1, GPR32:{ *:[i32] }:$src2, (imm:{ *:[iPTR] }):$src3)  =>  (INSERTELT:{ *:[v4i32] } VecReg128:{ *:[v4i32] }:$src1, GPR32:{ *:[i32] }:$src2, (imm:{ *:[i8] }):$src3)
+// NOOPT-NEXT:      GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::INSERTELT),
+// NOOPT-NEXT:      GIR_RootToRootCopy, /*OpIdx*/0, // DstI[dst]
+// NOOPT-NEXT:      GIR_RootToRootCopy, /*OpIdx*/1, // src1
+// NOOPT-NEXT:      GIR_RootToRootCopy, /*OpIdx*/2, // src2
+// NOOPT-NEXT:      GIR_CopyConstantAsSImm, /*NewInsnID*/0, /*OldInsnID*/1, // src3
+// NOOPT-NEXT:      GIR_RootConstrainSelectedInstOperands,
+// NOOPT-NEXT:      // GIR_Coverage, [[ID]],
+// NOOPT-NEXT:      GIR_EraseRootFromParent_Done,
+// NOOPT-NEXT:    // Label [[LABEL_NUM]]: @[[LABEL]]
+
+def INSERTELT : I<(outs VecReg128:$dst),
+               (ins VecReg128:$src1, GPR32:$src2, i8imm:$src3),
+               [(set VecReg128:$dst, (v4i32 (insertelt VecReg128:$src1, GPR32:$src2, imm:$src3)))]>;
+
 //===- Test a nested instruction match. -----------------------------------===//
 //
-// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
+// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
 // NOOPT-NEXT:    GIM_CheckFeatures, GIMT_Encode2(GIFBS_HasA),
 // NOOPT-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
 // NOOPT-NEXT:    GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_MUL),
@@ -791,13 +832,13 @@ def : Pat<(not GPR32:$Wm), (ORN R0, GPR32:$Wm)>;
 // NOOPT-NEXT:    GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/2, // src2
 // NOOPT-NEXT:    GIR_RootToRootCopy, /*OpIdx*/2, // src3
 // NOOPT-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// NOOPT-NEXT:    // GIR_Coverage, 7,
+// NOOPT-NEXT:    // GIR_Coverage, [[ID]],
 // NOOPT-NEXT:    GIR_EraseRootFromParent_Done,
 // NOOPT-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 
 // We also get a second rule by commutativity.
 //
-// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
+// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
 // NOOPT-NEXT:    GIM_CheckFeatures, GIMT_Encode2(GIFBS_HasA),
 // NOOPT-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
 // NOOPT-NEXT:    GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_MUL),
@@ -828,7 +869,7 @@ def : Pat<(not GPR32:$Wm), (ORN R0, GPR32:$Wm)>;
 // NOOPT-NEXT:    GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/2, // src2
 // NOOPT-NEXT:    GIR_RootToRootCopy, /*OpIdx*/1, // src3
 // NOOPT-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// NOOPT-NEXT:    // GIR_Coverage, 28,
+// NOOPT-NEXT:    // GIR_Coverage, [[ID]],
 // NOOPT-NEXT:    GIR_EraseRootFromParent_Done,
 // NOOPT-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 
@@ -839,7 +880,7 @@ def MULADD : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2, GPR32:$src3),
 
 //===- Test a simple pattern with just a specific leaf immediate. ---------===//
 //
-// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
+// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
 // NOOPT-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
 // NOOPT-NEXT:    GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_CONSTANT),
 // NOOPT-NEXT:    // MIs[0] DstI[dst]
@@ -851,7 +892,7 @@ def MULADD : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2, GPR32:$src3),
 // NOOPT-NEXT:    GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::MOV1),
 // NOOPT-NEXT:    GIR_RootToRootCopy, /*OpIdx*/0, //  DstI[dst]
 // NOOPT-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// NOOPT-NEXT:    // GIR_Coverage, 8,
+// NOOPT-NEXT:    // GIR_Coverage, [[ID]],
 // NOOPT-NEXT:    GIR_EraseRootFromParent_Done,
 // NOOPT-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 
@@ -859,7 +900,7 @@ def MOV1 : I<(outs GPR32:$dst), (ins), [(set GPR32:$dst, 1)]>;
 
 //===- Test a simple pattern with a leaf immediate and a predicate. -------===//
 //
-// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
+// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
 // NOOPT-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
 // NOOPT-NEXT:    GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_CONSTANT),
 // NOOPT-NEXT:    GIM_CheckI64ImmPredicate, /*MI*/0, /*Predicate*/GIMT_Encode2(GICXXPred_I64_Predicate_simm8),
@@ -873,7 +914,7 @@ def MOV1 : I<(outs GPR32:$dst), (ins), [(set GPR32:$dst, 1)]>;
 // NOOPT-NEXT:    GIR_RootToRootCopy, /*OpIdx*/0, //  DstI[dst]
 // NOOPT-NEXT:    GIR_CopyConstantAsSImm, /*NewInsnID*/0, /*OldInsnID*/0, // imm
 // NOOPT-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// NOOPT-NEXT:    // GIR_Coverage, 9,
+// NOOPT-NEXT:    // GIR_Coverage, [[ID]],
 // NOOPT-NEXT:    GIR_EraseRootFromParent_Done,
 // NOOPT-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 
@@ -882,7 +923,7 @@ def MOVimm8 : I<(outs GPR32:$dst), (ins i32imm:$imm), [(set GPR32:$dst, simm8:$i
 
 //===- Same again but use an IntImmLeaf. ----------------------------------===//
 //
-// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
+// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
 // NOOPT-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
 // NOOPT-NEXT:    GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_CONSTANT),
 // NOOPT-NEXT:    GIM_CheckAPIntImmPredicate, /*MI*/0, /*Predicate*/GIMT_Encode2(GICXXPred_APInt_Predicate_simm9),
@@ -896,7 +937,7 @@ def MOVimm8 : I<(outs GPR32:$dst), (ins i32imm:$imm), [(set GPR32:$dst, simm8:$i
 // NOOPT-NEXT:    GIR_RootToRootCopy, /*OpIdx*/0, //  DstI[dst]
 // NOOPT-NEXT:    GIR_CopyConstantAsSImm, /*NewInsnID*/0, /*OldInsnID*/0, // imm
 // NOOPT-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// NOOPT-NEXT:    // GIR_Coverage, 10,
+// NOOPT-NEXT:    // GIR_Coverage, [[ID]],
 // NOOPT-NEXT:    GIR_EraseRootFromParent_Done,
 // NOOPT-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 
@@ -905,7 +946,7 @@ def MOVimm9 : I<(outs GPR32:$dst), (ins i32imm:$imm), [(set GPR32:$dst, simm9:$i
 
 //===- Test a pattern with a custom renderer. -----------------------------===//
 //
-// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
+// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
 // NOOPT-NEXT:    GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
 // NOOPT-NEXT:    GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_CONSTANT),
 // NOOPT-NEXT:    GIM_CheckI64ImmPredicate, /*MI*/0, /*Predicate*/GIMT_Encode2(GICXXPred_I64_Predicate_cimm8),
@@ -919,7 +960,7 @@ def MOVimm9 : I<(outs GPR32:$dst), (ins i32imm:$imm), [(set GPR32:$dst, simm9:$i
 // NOOPT-NEXT:    GIR_RootToRootCopy, /*OpIdx*/0, //  DstI[dst]
 // NOOPT-NEXT:    GIR_CustomRenderer, /*InsnID*/0, /*OldInsnID*/0, /*Renderer*/GIMT_Encode2(GICR_renderImm), // imm
 // NOOPT-NEXT:    GIR_RootConstrainSelectedInstOperands,
-// NOOPT-NEXT:    // GIR_Coverage, 11,
+// NOOPT-NEXT:    // GIR_Coverage, [[ID]],
 // NOOPT-NEXT:    GIR_EraseRootFromParent_Done,
 // NOOPT-NEXT:  // Label [[LABEL_NUM]]: @[[LABEL]]
 
@@ -927,7 +968,7 @@ def MOVcimm8 : I<(outs GPR32:$dst), (ins i32imm:$imm), [(set GPR32:$dst, cimm8:$
 
 //===- Test a simple pattern with a FP immediate and a predicate. ---------===//
 //
-// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
+// NOOPT-NEXT:  GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]), // Rule ID [[ID:[0-9]+]]
 // NOO...
[truncated]

arsenm · 2024-10-08T09:54:22Z

It is dictated by the fact that insertelt node has SDTCisPtrTy

This is just a bug, the vector index type is separate from the pointer type

e-kud · 2024-12-28T14:22:20Z

This is just a bug, the vector index type is separate from the pointer type

We can't simply change the constraint in general

def SDTVecInsert : SDTypeProfile<1, 3, [    // vector insert
  SDTCisEltOfVec<2, 1>, SDTCisSameAs<0, 1>, SDTCisPtrTy<3>
]>;

Most of the target have already accepted the idea of iPTR index. To fix the bug, all targets should be fixed first.

Then I tried to change the element type to i32 only on X86 (we need some common type for 32bit and 64bit platforms). It caused a lot of regressions since we have some combines that assume only for i64 indices. Some of them are easy to fix, some of them aren't as different combines become active.

I'll try to solve them one by one before migrating to i32 index type. If anyone has better ideas on how to proceed, I'd appreciate it. Closing for now, as migration to i32 seems feasible.

e-kud · 2025-04-16T14:26:47Z

This is just a bug, the vector index type is separate from the pointer type

@arsenm if this is a bug it's already buried too deep.

/// Returns the type to be used for the index operand vector operations. By
/// default we assume it will have the same size as an address space 0
/// pointer.
virtual unsigned getVectorIdxWidth(const DataLayout &DL) const {
  return DL.getPointerSizeInBits(0);
}

/// Returns the type to be used for the index operand of:
/// ISD::INSERT_VECTOR_ELT, ISD::EXTRACT_VECTOR_ELT,
/// ISD::INSERT_SUBVECTOR, and ISD::EXTRACT_SUBVECTOR
MVT getVectorIdxTy(const DataLayout &DL) const {
  return MVT::getIntegerVT(getVectorIdxWidth(DL));
}

/// Returns the type to be used for the index operand of:
/// G_INSERT_VECTOR_ELT, G_EXTRACT_VECTOR_ELT,
/// G_INSERT_SUBVECTOR, and G_EXTRACT_SUBVECTOR
LLT getVectorIdxLLT(const DataLayout &DL) const {
  return LLT::scalar(getVectorIdxWidth(DL));
}

So each target that has vectors implemented in SDAG must overwrite default interface and whole SDAG implementation to enable vector patterns in GlobalISel. It looks like supporting iPTR as a HW dependent integer type could be more straightforward in terms of GlobalISel adoption. What do you think?

cc @aemerson @davemgreen @topperc

arsenm · 2025-04-23T11:04:59Z

This is just a bug, the vector index type is separate from the pointer type

@arsenm if this is a bug it's already buried too deep.

The default implementation happens to use the address space 0 pointer type, that's not very deep.

So each target that has vectors implemented in SDAG must overwrite default interface and whole SDAG implementation to enable vector patterns in GlobalISel.

I think we should just remove the SDTCisPtrTy constraint from the vector nodes. It's simply not correct. If we wanted to, we could have a parallel mechanism for the vector index type

e-kud · 2025-04-25T00:35:21Z

I think we should just remove the SDTCisPtrTy constraint from the vector nodes. It's simply not correct. If we wanted to, we could have a parallel mechanism for the vector index type

Yes, this is an entry point. After removing or replacing it with SDTCisIntTy we start seeing a problem that some patterns couldn't be generated because of Could not infer all types in pattern!. It happens because now several types are available for an operand. It is fixable for targets with one type of pointers: 32-bit or 64-bit, we can simply add i32 or i64 in patterns, not a problem.

For targets with multi-width pointers we need to set a type iPTR so it will always match the result of getVectorIdxTy as by default we return the width of the pointer type. Or we need to loop over all pointer widths.

Now, having iPTR we can't import patterns because we bail out with unsupported type from addTypeCheckPredicate.

About x86 in particular. Let's override getVectorIdxWidth, so we need to return i32 so it works with 32 and 64 bit platforms. When I've done it, there is a bunch of tests that start failing because some a row of combines stopped working as they were focused on 64 bit indices. I'm not sure whether it's worth it,

e-kud requested review from aemerson, arsenm, davemgreen and topperc October 8, 2024 08:55

llvmbot added tablegen llvm:globalisel labels Oct 8, 2024

Add missing comment for the matcher

1726989

e-kud closed this Dec 28, 2024

e-kud mentioned this pull request Apr 24, 2025

[X86][GlobalIsel] support G_FABS #136718

Merged

e-kud mentioned this pull request Jun 6, 2025

[X86][GISel] Add legalization support for non-constant G_BUILD_VECTOR #143099

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[GlobalISel] Enhance iPTR type support in SDAG patterns #111503

[GlobalISel] Enhance iPTR type support in SDAG patterns #111503

Uh oh!

e-kud commented Oct 8, 2024 •

edited

Loading

Uh oh!

llvmbot commented Oct 8, 2024 •

edited

Loading

Uh oh!

arsenm commented Oct 8, 2024

Uh oh!

e-kud commented Dec 28, 2024 •

edited

Loading

Uh oh!

e-kud commented Apr 16, 2025

Uh oh!

arsenm commented Apr 23, 2025

Uh oh!

e-kud commented Apr 25, 2025

Uh oh!

Uh oh!

[GlobalISel] Enhance iPTR type support in SDAG patterns #111503

[GlobalISel] Enhance iPTR type support in SDAG patterns #111503

Uh oh!

Conversation

e-kud commented Oct 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arsenm commented Oct 8, 2024

Uh oh!

e-kud commented Dec 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

e-kud commented Apr 16, 2025

Uh oh!

arsenm commented Apr 23, 2025

Uh oh!

e-kud commented Apr 25, 2025

Uh oh!

Uh oh!

e-kud commented Oct 8, 2024 •

edited

Loading

llvmbot commented Oct 8, 2024 •

edited

Loading

e-kud commented Dec 28, 2024 •

edited

Loading