Skip to content

Commit

Permalink
[GISel] Add RegState::Define to temporary defs in apply patterns (llv…
Browse files Browse the repository at this point in the history
…m#77425)

Previously, registers created for temporary defs in apply patterns were
rendered as uses, resulting in machine verifier errors.
  • Loading branch information
s-barannikov authored and justinfargnoli committed Jan 28, 2024
1 parent 71b28af commit e862455
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 19 deletions.
Expand Up @@ -28,11 +28,11 @@ def MyCombiner: GICombiner<"GenMyCombiner", [

// CHECK: const uint8_t *GenMyCombiner::getMatchTable() const {
// CHECK-NEXT: constexpr static uint8_t MatchTable0[] = {
// CHECK-NEXT: GIM_SwitchOpcode, /*MI*/0, /*[*/GIMT_Encode2(65), GIMT_Encode2(181), /*)*//*default:*//*Label 2*/ GIMT_Encode4(556),
// CHECK-NEXT: GIM_SwitchOpcode, /*MI*/0, /*[*/GIMT_Encode2(65), GIMT_Encode2(181), /*)*//*default:*//*Label 2*/ GIMT_Encode4(558),
// CHECK-NEXT: /*TargetOpcode::G_UNMERGE_VALUES*//*Label 0*/ GIMT_Encode4(474), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0), GIMT_Encode4(0),
// CHECK-NEXT: /*TargetOpcode::G_FNEG*//*Label 1*/ GIMT_Encode4(524),
// CHECK-NEXT: /*TargetOpcode::G_FNEG*//*Label 1*/ GIMT_Encode4(526),
// CHECK-NEXT: // Label 0: @474
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 3*/ GIMT_Encode4(523), // Rule ID 1 //
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 3*/ GIMT_Encode4(525), // Rule ID 1 //
// CHECK-NEXT: GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule1Enabled),
// CHECK-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/3,
// CHECK-NEXT: // MIs[0] a
Expand All @@ -52,15 +52,15 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK-NEXT: // Combiner Rule #1: ReplaceTemp
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/GIMT_Encode2(TargetOpcode::G_UNMERGE_VALUES),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // a
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/2, // y
// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,
// CHECK-NEXT: GIR_ReplaceRegWithTempReg, /*OldInsnID*/0, /*OldOpIdx*/1, /*TempRegID*/0,
// CHECK-NEXT: GIR_Done,
// CHECK-NEXT: // Label 3: @523
// CHECK-NEXT: // Label 3: @525
// CHECK-NEXT: GIM_Reject,
// CHECK-NEXT: // Label 1: @524
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 4*/ GIMT_Encode4(555), // Rule ID 0 //
// CHECK-NEXT: // Label 1: @526
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 4*/ GIMT_Encode4(557), // Rule ID 0 //
// CHECK-NEXT: GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
// CHECK-NEXT: // MIs[0] dst
// CHECK-NEXT: // No operand predicates
Expand All @@ -75,10 +75,10 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK-NEXT: GIR_ReplaceReg, /*OldInsnID*/0, /*OldOpIdx*/0, /*NewInsnId*/1, /*NewOpIdx*/1,
// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,
// CHECK-NEXT: GIR_Done,
// CHECK-NEXT: // Label 4: @555
// CHECK-NEXT: // Label 4: @557
// CHECK-NEXT: GIM_Reject,
// CHECK-NEXT: // Label 2: @556
// CHECK-NEXT: // Label 2: @558
// CHECK-NEXT: GIM_Reject,
// CHECK-NEXT: }; // Size: 557 bytes
// CHECK-NEXT: }; // Size: 559 bytes
// CHECK-NEXT: return MatchTable0;
// CHECK-NEXT: }
Expand Up @@ -21,7 +21,7 @@ def MyCombiner: GICombiner<"GenMyCombiner", [

// CHECK: const uint8_t *GenMyCombiner::getMatchTable() const {
// CHECK-NEXT: constexpr static uint8_t MatchTable0[] = {
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(79), // Rule ID 0 //
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(81), // Rule ID 0 //
// CHECK-NEXT: GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_MUL),
// CHECK-NEXT: GIM_CheckType, /*MI*/0, /*Op*/0, /*Type*/GILLT_s8,
Expand All @@ -36,7 +36,7 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s64,
// CHECK-NEXT: // Combiner Rule #0: InstTest0
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/GIMT_Encode2(TargetOpcode::G_ADD),
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/1, // b
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/1, /*OpIdx*/2, // c
// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,
Expand All @@ -45,8 +45,8 @@ def MyCombiner: GICombiner<"GenMyCombiner", [
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/1, // b
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/1, /*TempRegID*/0,
// CHECK-NEXT: GIR_Done,
// CHECK-NEXT: // Label 0: @79
// CHECK-NEXT: // Label 0: @81
// CHECK-NEXT: GIM_Reject,
// CHECK-NEXT: };
// CHECK-NEXT: }; // Size: 82 bytes
// CHECK-NEXT: return MatchTable0;
// CHECK-NEXT: }
@@ -0,0 +1,56 @@
// RUN: llvm-tblgen -I %p/../../../include -gen-global-isel-combiner \
// RUN: -combiners=MyCombiner %s | FileCheck %s

// Checks that temporary registers defined in apply patterns
// are emitted with RegState::Define.

include "llvm/Target/Target.td"
include "llvm/Target/GlobalISel/Combine.td"

def MyTargetISA : InstrInfo;
def MyTarget : Target { let InstructionSet = MyTargetISA; }

def Test0 : GICombineRule<
(defs root:$dst),
(match (G_ADD $dst, $lhs, $rhs)),
(apply (G_UDIVREM $tmp, $dst, $lhs, $rhs))
>;

def Test1 : GICombineRule<
(defs root:$dst),
(match (G_ADD $dst, $lhs, $rhs)),
(apply (G_UDIVREM $dst, $tmp, $lhs, $rhs))
>;

def Test2 : GICombineRule<
(defs root:$dst),
(match (G_ADD $dst, $lhs, $rhs)),
(apply (G_ADD $tmp, 0, $lhs),
(G_ADD $dst, $tmp, $rhs))
>;

def MyCombiner: GICombiner<"GenMyCombiner", [
Test0,
Test1,
Test2,
]>;

// CHECK: // Combiner Rule #0: Test0
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/GIMT_Encode2(TargetOpcode::G_UDIVREM),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/1, // lhs
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/2, // rhs

// CHECK: // Combiner Rule #1: Test1
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/GIMT_Encode2(TargetOpcode::G_UDIVREM),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/1, // lhs
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/2, // rhs

// CHECK: // Combiner Rule #2: Test2
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/GIMT_Encode2(TargetOpcode::G_ADD),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/1,
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/1, // lhs
Expand Up @@ -16,7 +16,7 @@ def Test0 : GICombineRule<

// CHECK: const uint8_t *GenMyCombiner::getMatchTable() const {
// CHECK-NEXT: constexpr static uint8_t MatchTable0[] = {
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(75), // Rule ID 0 //
// CHECK-NEXT: GIM_Try, /*On fail goto*//*Label 0*/ GIMT_Encode4(77), // Rule ID 0 //
// CHECK-NEXT: GIM_CheckSimplePredicate, GIMT_Encode2(GICXXPred_Simple_IsRule0Enabled),
// CHECK-NEXT: GIM_CheckOpcode, /*MI*/0, GIMT_Encode2(TargetOpcode::G_MUL),
// CHECK-NEXT: // MIs[0] dst
Expand All @@ -30,17 +30,17 @@ def Test0 : GICombineRule<
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/uint8_t(-1),
// CHECK-NEXT: // Combiner Rule #0: Test0
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/GIMT_Encode2(TargetOpcode::G_CONSTANT),
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_AddCImm, /*InsnID*/0, /*Type*/uint8_t(-2), /*Imm*/GIMT_Encode8(42),
// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::G_SUB),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/0, // dst
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/1, /*TempRegID*/1,
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/1, /*TempRegID*/0,
// CHECK-NEXT: GIR_Done,
// CHECK-NEXT: // Label 0: @75
// CHECK-NEXT: // Label 0: @77
// CHECK-NEXT: GIM_Reject,
// CHECK-NEXT: }; // Size: 76 bytes
// CHECK-NEXT: }; // Size: 78 bytes
// CHECK-NEXT: return MatchTable0;
// CHECK-NEXT: }

Expand Down
2 changes: 1 addition & 1 deletion llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
Expand Up @@ -2318,7 +2318,7 @@ bool CombineRuleBuilder::emitInstructionApplyPattern(
M.actions_begin(), getLLTCodeGenOrTempType(Ty, M), TempRegID);
}

DstMI.addRenderer<TempRegRenderer>(TempRegID);
DstMI.addRenderer<TempRegRenderer>(TempRegID, /*IsDef=*/true);
}

// Render MIFlags
Expand Down

0 comments on commit e862455

Please sign in to comment.