Skip to content

Commit bda7aad

Browse files
s-barannikovs-barannikov
authored
[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).
1 parent aca7a70 commit bda7aad

File tree

3 files changed

+66
-32
lines changed

3 files changed

+66
-32
lines changed

‎llvm/include/llvm/Target/GenericOpcodes.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ def G_PHI : GenericInstruction {
9999
}
100100

101101
def G_FRAME_INDEX : GenericInstruction {
102-
let OutOperandList = (outs type0:$dst);
102+
let OutOperandList = (outs ptype0:$dst);
103103
let InOperandList = (ins unknown:$src2);
104104
let hasSideEffects = false;
105105
}

‎llvm/test/TableGen/GlobalISelEmitter-frameindex.td

Lines changed: 63 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,67 @@
1-
// RUN: llvm-tblgen -gen-global-isel -optimize-match-table=false -I %p/../../include -I %p/Common %s -o - < %s | FileCheck %s
1+
// RUN: llvm-tblgen -gen-global-isel -optimize-match-table=false -I %p/../../include -I %p/Common %s | FileCheck %s
22

33
include "llvm/Target/Target.td"
44
include "GlobalISelEmitterCommon.td"
55

6-
def ADD : I<(outs GPR32:$dst), (ins GPR32:$src1, GPR32:$src2), []>;
7-
8-
//===- Test a simple pattern with frame index operands. ----------------------===//
9-
//
10-
// CHECK: GIM_Try, /*On fail goto*//*Label [[LABEL_NUM:[0-9]+]]*/ GIMT_Encode4([[LABEL:[0-9]+]]),
11-
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
12-
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_FRAME_INDEX),
13-
// CHECK-NEXT: // MIs[0] DstI[dst]
14-
// CHECK-NEXT: GIM_RootCheckType, /*Op*/0, /*Type*/GILLT_p0s32,
15-
// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
16-
// CHECK-NEXT: // MIs[0] fi
17-
// CHECK-NEXT: // No operand predicates
18-
// CHECK-NEXT: // (frameindex:{ *:[i32] }):$fi => (ADD:{ *:[i32] } (tframeindex:{ *:[i32] }):$fi, 0:{ *:[i32] })
19-
// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::ADD),
20-
// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[dst]
21-
// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/1, // fi
22-
// CHECK-NEXT: GIR_AddImm8, /*InsnID*/0, /*Imm*/0,
23-
// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
24-
// CHECK-NEXT: // GIR_Coverage, 0,
25-
// CHECK-NEXT: GIR_EraseRootFromParent_Done,
26-
// CHECK-NEXT: // Label [[LABEL_NUM]]: @[[LABEL]]
27-
// CHECK-NEXT: GIM_Reject,
28-
29-
def : Pat<(p0 frameindex:$fi), (ADD tframeindex:$fi, 0)>;
6+
def ADDI : I<(outs GPR32:$dst), (ins GPR32:$src1, i32imm:$src2), []>;
7+
8+
def to_tframeindex : SDNodeXForm<frameindex, [{}]>;
9+
10+
def : GICustomOperandRenderer<"renderFrameIndex">,
11+
GISDNodeXFormEquiv<to_tframeindex>;
12+
13+
def : Pat<(frameindex:$fi), (ADDI (to_tframeindex $fi), 0)>;
14+
15+
def : Pat<(ptradd frameindex:$fi, (i32 imm:$offset)),
16+
(ADDI (to_tframeindex $fi), imm:$offset)>;
17+
18+
// CHECK-LABEL: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(74), // Rule ID 1 //
19+
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
20+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_PTR_ADD),
21+
// CHECK-NEXT: // MIs[0] DstI[dst]
22+
// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/0, /*SizeInBits*/32,
23+
// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
24+
// CHECK-NEXT: // MIs[0] fi
25+
// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/1, /*SizeInBits*/32,
26+
// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/1, /*MI*/0, /*OpIdx*/1, // MIs[1]
27+
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/1, /*Expected*/2,
28+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/1, GIMT_Encode2(TargetOpcode::G_FRAME_INDEX),
29+
// CHECK-NEXT: // MIs[1] Operand 0
30+
// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/1, /*Op*/0, /*SizeInBits*/32,
31+
// CHECK-NEXT: // MIs[1] Operand 1
32+
// CHECK-NEXT: // No operand predicates
33+
// CHECK-NEXT: // MIs[0] offset
34+
// CHECK-NEXT: GIM_RootCheckType, /*Op*/2, /*Type*/GILLT_s32,
35+
// CHECK-NEXT: GIM_RecordInsn, /*DefineMI*/2, /*MI*/0, /*OpIdx*/2, // MIs[2]
36+
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/2, /*Expected*/2,
37+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/2, GIMT_Encode2(TargetOpcode::G_CONSTANT),
38+
// CHECK-NEXT: // MIs[2] Operand 0
39+
// CHECK-NEXT: GIM_CheckType, /*MI*/2, /*Op*/0, /*Type*/GILLT_s32,
40+
// CHECK-NEXT: // MIs[2] Operand 1
41+
// CHECK-NEXT: // No operand predicates
42+
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*NumInsns*/2,
43+
// CHECK-NEXT: // (ptradd:{ *:[i32] } (frameindex:{ *:[i32] }):$fi, (imm:{ *:[i32] }):$offset) => (ADDI:{ *:[i32] } (to_tframeindex:{ *:[i32] } ?:{ *:[i32] }:$fi), (imm:{ *:[i32] }):$offset)
44+
// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::ADDI),
45+
// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[dst]
46+
// CHECK-NEXT: GIR_CustomRenderer, /*InsnID*/0, /*OldInsnID*/1, /*Renderer*/GIMT_Encode2(GICR_renderFrameIndex), // fi
47+
// CHECK-NEXT: GIR_CopyConstantAsSImm, /*NewInsnID*/0, /*OldInsnID*/2, // offset
48+
// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
49+
// CHECK-NEXT: // GIR_Coverage, 1,
50+
// CHECK-NEXT: GIR_EraseRootFromParent_Done,
51+
52+
// CHECK-LABEL: GIM_Try, /*On fail goto*//*Label 1*/ GIMT_Encode4(109), // Rule ID 0 //
53+
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/2,
54+
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_FRAME_INDEX),
55+
// CHECK-NEXT: // MIs[0] DstI[dst]
56+
// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/0, /*SizeInBits*/32,
57+
// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
58+
// CHECK-NEXT: // MIs[0] Operand 1
59+
// CHECK-NEXT: // No operand predicates
60+
// CHECK-NEXT: // (frameindex:{ *:[i32] }):$fi => (ADDI:{ *:[i32] } (to_tframeindex:{ *:[i32] } ?:{ *:[i32] }:$fi), 0:{ *:[i32] })
61+
// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::ADDI),
62+
// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[dst]
63+
// CHECK-NEXT: GIR_CustomRenderer, /*InsnID*/0, /*OldInsnID*/0, /*Renderer*/GIMT_Encode2(GICR_renderFrameIndex), // fi
64+
// CHECK-NEXT: GIR_AddImm8, /*InsnID*/0, /*Imm*/0,
65+
// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
66+
// CHECK-NEXT: // GIR_Coverage, 0,
67+
// CHECK-NEXT: GIR_EraseRootFromParent_Done,

‎llvm/utils/TableGen/GlobalISelEmitter.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -836,19 +836,15 @@ Expected<InstructionMatcher &> GlobalISelEmitter::createAndImportSelDAGMatcher(
836836
assert(SrcGIOrNull &&
837837
"Expected to have already found an equivalent Instruction");
838838
if (SrcGIOrNull->TheDef->getName() == "G_CONSTANT" ||
839-
SrcGIOrNull->TheDef->getName() == "G_FCONSTANT") {
839+
SrcGIOrNull->TheDef->getName() == "G_FCONSTANT" ||
840+
SrcGIOrNull->TheDef->getName() == "G_FRAME_INDEX") {
840841
// imm/fpimm still have operands but we don't need to do anything with it
841842
// here since we don't support ImmLeaf predicates yet. However, we still
842843
// need to note the hidden operand to get GIM_CheckNumOperands correct.
843844
InsnMatcher.addOperand(OpIdx++, "", TempOpIdx);
844845
return InsnMatcher;
845846
}
846847

847-
if (SrcGIOrNull->TheDef->getName() == "G_FRAME_INDEX") {
848-
InsnMatcher.addOperand(OpIdx++, Src.getName(), TempOpIdx);
849-
return InsnMatcher;
850-
}
851-
852848
// Special case because the operand order is changed from setcc. The
853849
// predicate operand needs to be swapped from the last operand to the first
854850
// source.

0 commit comments

Comments
 (0)