[llvm] [TableGen][GISel] Import frameindex correctly (PR #120921)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Dec 22 14:56:48 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-tablegen
Author: Sergei Barannikov (s-barannikov)
<details>
<summary>Changes</summary>
The support added in #<!-- -->90475 was inherently broken.
The pattern in the added test is incorrect, and even though TableGen doesn't complain, the code generated from it is invalid and fails verification with the message "Use not jointly dominated by defs.".
There is no way to magically transform `frameindex` to `tframeindex` as it happens for some other leaf nodes. `frameindex` can only be selected by custom C++ code or by using an `SDNodeXForm`.
This patch removes special handling of `tframeindex` so that the pattern becomes unimportable, and fixes the handling of G_FRAME_INDEX, which shouldn't have set the operand's name.
It also fixes the type of the result of G_FRAME_INDEX in order to get the correct type check (GIM_CheckPointerToAny instead of GIM_CheckType with scalar LLT argument).
---
Full diff: https://github.com/llvm/llvm-project/pull/120921.diff
3 Files Affected:
- (modified) llvm/include/llvm/Target/GenericOpcodes.td (+1-1)
- (modified) llvm/test/TableGen/GlobalISelEmitter-frameindex.td (+63-25)
- (modified) llvm/utils/TableGen/GlobalISelEmitter.cpp (+2-10)
``````````diff
diff --git a/llvm/include/llvm/Target/GenericOpcodes.td b/llvm/include/llvm/Target/GenericOpcodes.td
index c8f91cd0de5978..e134bab61bf63c 100644
--- a/llvm/include/llvm/Target/GenericOpcodes.td
+++ b/llvm/include/llvm/Target/GenericOpcodes.td
@@ -99,7 +99,7 @@ def G_PHI : GenericInstruction {
}
def G_FRAME_INDEX : GenericInstruction {
- let OutOperandList = (outs type0:$dst);
+ let OutOperandList = (outs ptype0:$dst);
let InOperandList = (ins unknown:$src2);
let hasSideEffects = false;
}
diff --git a/llvm/test/TableGen/GlobalISelEmitter-frameindex.td b/llvm/test/TableGen/GlobalISelEmitter-frameindex.td
index 232691465bb3bd..715e53ddbad08d 100644
--- a/llvm/test/TableGen/GlobalISelEmitter-frameindex.td
+++ b/llvm/test/TableGen/GlobalISelEmitter-frameindex.td
@@ -1,29 +1,67 @@
-// RUN: llvm-tblgen -gen-global-isel -optimize-match-table=false -I %p/../../include -I %p/Common %s -o - < %s | FileCheck %s
+// RUN: llvm-tblgen -gen-global-isel -optimize-match-table=false -I %p/../../include -I %p/Common %s | FileCheck %s
include "llvm/Target/Target.td"
include "GlobalISelEmitterCommon.td"
-def ADD : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2), []>;
-
-//===- Test a simple pattern with frame index operands. ----------------------===//
-//
-// CHECK: GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
-// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
-// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_FRAME_INDEX),
-// CHECK-NEXT: // MIs[0] DstI[dst]
-// CHECK-NEXT: GIM_RootCheckType, /*Op*/0, /*Type*/GILLT_p0s32,
-// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
-// CHECK-NEXT: // MIs[0] fi
-// CHECK-NEXT: // No operand predicates
-// CHECK-NEXT: // (frameindex:{ *:[i32] }):$fi => (ADD:{ *:[i32] } (tframeindex:{ *:[i32] }):$fi, 0:{ *:[i32] })
-// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::ADD),
-// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[dst]
-// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/1, // fi
-// CHECK-NEXT: GIR_AddImm8, /*InsnID*/0, /*Imm*/0,
-// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
-// CHECK-NEXT: // GIR_Coverage, 0,
-// CHECK-NEXT: GIR_EraseRootFromParent_Done,
-// CHECK-NEXT: // Label [[LABEL_NUM]]: @[[LABEL]]
-// CHECK-NEXT: GIM_Reject,
-
-def : Pat<(p0 frameindex:$fi), (ADD tframeindex:$fi, 0)>;
+def ADDI : I<(outs GPR32:$dst), (ins GPR32:$src1, i32imm:$src2), []>;
+
+def to_tframeindex : SDNodeXForm<frameindex, [{}]>;
+
+def : GICustomOperandRenderer<"renderFrameIndex">,
+ GISDNodeXFormEquiv<to_tframeindex>;
+
+def : Pat<(frameindex:$fi), (ADDI (to_tframeindex $fi), 0)>;
+
+def : Pat<(ptradd frameindex:$fi, (i32 imm:$offset)),
+ (ADDI (to_tframeindex $fi), imm:$offset)>;
+
+// CHECK-LABEL: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(74), // Rule ID 1 //
+// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_PTR_ADD),
+// CHECK-NEXT: // MIs[0] DstI[dst]
+// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/0, /*SizeInBits*/32,
+// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
+// CHECK-NEXT: // MIs[0] fi
+// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/1, /*SizeInBits*/32,
+// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
+// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/1, /*Expected*/2,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, GIMT_Encode2(TargetOpcode::G_FRAME_INDEX),
+// CHECK-NEXT: // MIs[1] Operand 0
+// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/1, /*Op*/0, /*SizeInBits*/32,
+// CHECK-NEXT: // MIs[1] Operand 1
+// CHECK-NEXT: // No operand predicates
+// CHECK-NEXT: // MIs[0] offset
+// CHECK-NEXT: GIM_RootCheckType, /*Op*/2, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/2, /*MI*/0, /*OpIdx*/2, // MIs[2]
+// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/2, /*Expected*/2,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/2, GIMT_Encode2(TargetOpcode::G_CONSTANT),
+// CHECK-NEXT: // MIs[2] Operand 0
+// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/0, /*Type*/GILLT_s32,
+// CHECK-NEXT: // MIs[2] Operand 1
+// CHECK-NEXT: // No operand predicates
+// CHECK-NEXT: GIM_CheckIsSafeToFold, /*NumInsns*/2,
+// CHECK-NEXT: // (ptradd:{ *:[i32] } (frameindex:{ *:[i32] }):$fi, (imm:{ *:[i32] }):$offset) => (ADDI:{ *:[i32] } (to_tframeindex:{ *:[i32] } ?:{ *:[i32] }:$fi), (imm:{ *:[i32] }):$offset)
+// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::ADDI),
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[dst]
+// CHECK-NEXT: GIR_CustomRenderer, /*InsnID*/0, /*OldInsnID*/1, /*Renderer*/GIMT_Encode2(GICR_renderFrameIndex), // fi
+// CHECK-NEXT: GIR_CopyConstantAsSImm, /*NewInsnID*/0, /*OldInsnID*/2, // offset
+// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
+// CHECK-NEXT: // GIR_Coverage, 1,
+// CHECK-NEXT: GIR_EraseRootFromParent_Done,
+
+// CHECK-LABEL: GIM_Try, /*On fail goto*//*Label 1*/ GIMT_Encode4(109), // Rule ID 0 //
+// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
+// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_FRAME_INDEX),
+// CHECK-NEXT: // MIs[0] DstI[dst]
+// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/0, /*SizeInBits*/32,
+// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
+// CHECK-NEXT: // MIs[0] Operand 1
+// CHECK-NEXT: // No operand predicates
+// CHECK-NEXT: // (frameindex:{ *:[i32] }):$fi => (ADDI:{ *:[i32] } (to_tframeindex:{ *:[i32] } ?:{ *:[i32] }:$fi), 0:{ *:[i32] })
+// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::ADDI),
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[dst]
+// CHECK-NEXT: GIR_CustomRenderer, /*InsnID*/0, /*OldInsnID*/0, /*Renderer*/GIMT_Encode2(GICR_renderFrameIndex), // fi
+// CHECK-NEXT: GIR_AddImm8, /*InsnID*/0, /*Imm*/0,
+// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
+// CHECK-NEXT: // GIR_Coverage, 0,
+// CHECK-NEXT: GIR_EraseRootFromParent_Done,
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index c1a626fe299045..16ccab34d23577 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -836,7 +836,8 @@ Expected<InstructionMatcher &> GlobalISelEmitter::createAndImportSelDAGMatcher(
assert(SrcGIOrNull &&
"Expected to have already found an equivalent Instruction");
if (SrcGIOrNull->TheDef->getName() == "G_CONSTANT" ||
- SrcGIOrNull->TheDef->getName() == "G_FCONSTANT") {
+ SrcGIOrNull->TheDef->getName() == "G_FCONSTANT" ||
+ SrcGIOrNull->TheDef->getName() == "G_FRAME_INDEX") {
// imm/fpimm still have operands but we don't need to do anything with it
// here since we don't support ImmLeaf predicates yet. However, we still
// need to note the hidden operand to get GIM_CheckNumOperands correct.
@@ -844,11 +845,6 @@ Expected<InstructionMatcher &> GlobalISelEmitter::createAndImportSelDAGMatcher(
return InsnMatcher;
}
- if (SrcGIOrNull->TheDef->getName() == "G_FRAME_INDEX") {
- InsnMatcher.addOperand(OpIdx++, Src.getName(), TempOpIdx);
- return InsnMatcher;
- }
-
// Special case because the operand order is changed from setcc. The
// predicate operand needs to be swapped from the last operand to the first
// source.
@@ -1248,10 +1244,6 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitUseRenderer(
DstMIBuilder.addRenderer<CopyRenderer>(Dst.getName());
return InsertPt;
}
- if (Dst.getOperator()->getName() == "tframeindex") {
- DstMIBuilder.addRenderer<CopyRenderer>(Dst.getName());
- return InsertPt;
- }
if (Dst.getOperator()->getName() == "imm") {
DstMIBuilder.addRenderer<CopyConstantAsImmRenderer>(Dst.getName());
return InsertPt;
``````````
</details>
https://github.com/llvm/llvm-project/pull/120921
More information about the llvm-commits
mailing list