[llvm] bda7aad - [TableGen][GISel] Fix importing frameindex node (#120921)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 23 00:04:45 PST 2024
Author: Sergei Barannikov
Date: 2024-12-23T11:04:40+03:00
New Revision: bda7aadfcd0596675c8b4b381f178f3d8f854f20
URL: https://github.com/llvm/llvm-project/commit/bda7aadfcd0596675c8b4b381f178f3d8f854f20
DIFF: https://github.com/llvm/llvm-project/commit/bda7aadfcd0596675c8b4b381f178f3d8f854f20.diff
LOG: [TableGen][GISel] Fix importing frameindex node (#120921)
The existing test case is not representative. 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 makes the test representative one 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 a scalar LLT argument).
Added:
Modified:
llvm/include/llvm/Target/GenericOpcodes.td
llvm/test/TableGen/GlobalISelEmitter-frameindex.td
llvm/utils/TableGen/GlobalISelEmitter.cpp
Removed:
################################################################################
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..4250b57581f63e 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.
More information about the llvm-commits
mailing list