Skip to content

Commit d375041

Browse files
s-barannikovs-barannikov
authored
[TableGen][GISel] Improve dead register handling (#120426)
A dead implicit def wasn't marked as dead if it is also an implicit use. The new approach should also be more straightforward and simplifies future changes for supporting optional defs and physical register defs. Pull Request: #120426
1 parent aadf606 commit d375041

File tree

5 files changed

+35
-56
lines changed

5 files changed

+35
-56
lines changed

‎llvm/test/CodeGen/X86/GlobalISel/select-intrinsic-x86-flags-read-u32.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
define void @read_flags() { ret void }
1010
; CHECK-LABEL: name: read_flags
1111
; CHECK: bb.0:
12-
; CHECK: [[RDFLAGS32_:%[0-9]+]]:gr32 = RDFLAGS32 implicit-def $esp, implicit $esp
12+
; CHECK: [[RDFLAGS32_:%[0-9]+]]:gr32 = RDFLAGS32 implicit-def dead $esp, implicit $esp
1313
; CHECK: $eax = COPY [[RDFLAGS32_]]
1414
...
1515

‎llvm/test/TableGen/GlobalISelEmitter-nested-subregs.td‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ def A0 : RegisterClass<"MyTarget", [i32], 32, (add a0)>;
3838
// CHECK-NEXT: // MIs[0] src
3939
// CHECK-NEXT: GIM_RootCheckType, /*Op*/1, /*Type*/GILLT_s8,
4040
// CHECK-NEXT: // (anyext:{ *:[i16] } i8:{ *:[i8] }:$src) => (EXTRACT_SUBREG:{ *:[i16] } (INSERT_SUBREG:{ *:[i32] } (IMPLICIT_DEF:{ *:[i32] }), A0b:{ *:[i8] }:$src, lo8:{ *:[i32] }), lo16:{ *:[i32] })
41-
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
4241
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
4342
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(TargetOpcode::IMPLICIT_DEF),
4443
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
4544
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
45+
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
4646
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::INSERT_SUBREG),
4747
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
4848
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/1, /*TempRegID*/1,

‎llvm/test/TableGen/GlobalISelEmitterRegSequence.td‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@ def SUBSOME_INSN : I<(outs SRegs:$dst), (ins SOP:$src), []>;
3939
// CHECK-NEXT: GIM_RootCheckType, /*Op*/1, /*Type*/GILLT_s16,
4040
// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/1, /*RC*/GIMT_Encode2(Test::SRegsRegClassID),
4141
// CHECK-NEXT: // (sext:{ *:[i32] } SOP:{ *:[i16] }:$src) => (REG_SEQUENCE:{ *:[i32] } DRegs:{ *:[i32] }, (SUBSOME_INSN:{ *:[i16] } SOP:{ *:[i16] }:$src), sub0:{ *:[i32] }, (SUBSOME_INSN:{ *:[i16] } SOP:{ *:[i16] }:$src), sub1:{ *:[i32] })
42-
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s16,
4342
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s16,
4443
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(MyTarget::SUBSOME_INSN),
4544
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
4645
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/2, /*OldInsnID*/0, /*OpIdx*/1, // src
4746
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
47+
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s16,
4848
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(MyTarget::SUBSOME_INSN),
4949
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
5050
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/1, // src

‎llvm/test/TableGen/GlobalISelEmitterSubreg.td‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,13 @@ def : Pat<(sub (complex DOP:$src1, DOP:$src2), 77),
5959
(SOME_INSN2 (EXTRACT_SUBREG DOP:$src1, sub0),
6060
(EXTRACT_SUBREG DOP:$src2, sub1))>;
6161
// CHECK-LABEL: // (sub:{ *:[i32] } (complex:{ *:[i32] } DOP:{ *:[i32] }:$src1, DOP:{ *:[i32] }:$src2), 77:{ *:[i32] }) => (SOME_INSN2:{ *:[i32] } (EXTRACT_SUBREG:{ *:[i32] } DOP:{ *:[i32] }:$src1, sub0:{ *:[i32] }), (EXTRACT_SUBREG:{ *:[i32] } DOP:{ *:[i32] }:$src2, sub1:{ *:[i32] }))
62-
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
6362
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
6463
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
6564
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
6665
// CHECK-NEXT: GIR_ComplexSubOperandSubRegRenderer, /*InsnID*/2, /*RendererID*/GIMT_Encode2(0), /*SubOperand*/1, /*SubRegIdx*/GIMT_Encode2(2), // src2
6766
// CHECK-NEXT: GIR_ConstrainOperandRC, /*InsnID*/2, /*Op*/0, GIMT_Encode2(Test::SRegsRegClassID),
6867
// CHECK-NEXT: GIR_ConstrainOperandRC, /*InsnID*/2, /*Op*/1, GIMT_Encode2(Test::DRegsRegClassID),
68+
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
6969
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
7070
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
7171
// CHECK-NEXT: GIR_ComplexSubOperandSubRegRenderer, /*InsnID*/1, /*RendererID*/GIMT_Encode2(0), /*SubOperand*/0, /*SubRegIdx*/GIMT_Encode2(1), // src1
@@ -103,11 +103,11 @@ def : Pat<(i32 (anyext i16:$src)), (INSERT_SUBREG (i32 (IMPLICIT_DEF)), SOP:$src
103103
// instruction.
104104
def : Pat<(i32 (anyext i16:$src)), (SOME_INSN (INSERT_SUBREG (i32 (IMPLICIT_DEF)), SOP:$src, sub0))>;
105105
// CHECK-LABEL: (anyext:{ *:[i32] } i16:{ *:[i16] }:$src) => (SOME_INSN:{ *:[i32] } (INSERT_SUBREG:{ *:[i32] } (IMPLICIT_DEF:{ *:[i32] }), SOP:{ *:[i16] }:$src, sub0:{ *:[i32] }))
106-
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
107106
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
108107
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(TargetOpcode::IMPLICIT_DEF),
109108
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
110109
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
110+
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
111111
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::INSERT_SUBREG),
112112
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
113113
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/1, /*TempRegID*/1,
@@ -138,12 +138,12 @@ def : Pat<(i32 (anyext i16:$src)), (INSERT_SUBREG (i32 (COPY_TO_REGCLASS SOP:$sr
138138
// by a subinstruction.
139139
def : Pat<(i32 (anyext i16:$src)), (INSERT_SUBREG (i32 (IMPLICIT_DEF)), (SUBSOME_INSN SOP:$src), sub0)>;
140140
// CHECK-LABEL: (anyext:{ *:[i32] } i16:{ *:[i16] }:$src) => (INSERT_SUBREG:{ *:[i32] } (IMPLICIT_DEF:{ *:[i32] }), (SUBSOME_INSN:{ *:[i16] } SOP:{ *:[i16] }:$src), sub0:{ *:[i32] })
141-
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
142141
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s16,
143142
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(MyTarget::SUBSOME_INSN),
144143
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
145144
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/2, /*OldInsnID*/0, /*OpIdx*/1, // src
146145
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
146+
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
147147
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::IMPLICIT_DEF),
148148
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
149149
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/1,
@@ -200,12 +200,12 @@ def : Pat<(i16 (trunc (bitreverse DOP:$src))),
200200
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/1, /*RC*/GIMT_Encode2(Test::DRegsRegClassID),
201201
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*NumInsns*/1,
202202
// CHECK-NEXT: // (trunc:{ *:[i16] } (ctpop:{ *:[i32] } DOP:{ *:[i32] }:$src)) => (SUBSOME_INSN2:{ *:[i16] } (EXTRACT_SUBREG:{ *:[i16] } (SOME_INSN:{ *:[i32] } DOP:{ *:[i32] }:$src), sub0:{ *:[i32] }))
203-
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s16,
204203
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
205204
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(MyTarget::SOME_INSN),
206205
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
207206
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/2, /*OldInsnID*/1, /*OpIdx*/1, // src
208207
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
208+
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s16,
209209
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
210210
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
211211
// CHECK-NEXT: GIR_AddTempSubRegister, /*InsnID*/1, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(0), GIMT_Encode2(sub0),

‎llvm/utils/TableGen/GlobalISelEmitter.cpp‎

Lines changed: 28 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,6 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
324324
void emitTestSimplePredicate(raw_ostream &OS) override;
325325
void emitRunCustomAction(raw_ostream &OS) override;
326326

327-
void postProcessRule(RuleMatcher &M);
328-
329327
const CodeGenTarget &getTarget() const override { return Target; }
330328
StringRef getClassName() const override { return ClassName; }
331329

@@ -1410,12 +1408,10 @@ GlobalISelEmitter::createAndImportSubInstructionRenderer(
14101408
DstMIBuilder.addRenderer<TempRegRenderer>(TempRegID, true);
14111409

14121410
// Handle additional (ignored) results.
1413-
if (DstMIBuilder.getCGI()->Operands.NumDefs > 1) {
1414-
InsertPtOrError = importExplicitDefRenderers(
1415-
std::prev(*InsertPtOrError), M, DstMIBuilder, Src, Dst, /*Start=*/1);
1416-
if (auto Error = InsertPtOrError.takeError())
1417-
return std::move(Error);
1418-
}
1411+
InsertPtOrError = importExplicitDefRenderers(
1412+
std::prev(*InsertPtOrError), M, DstMIBuilder, Src, Dst, /*Start=*/1);
1413+
if (auto Error = InsertPtOrError.takeError())
1414+
return std::move(Error);
14191415

14201416
InsertPtOrError = importExplicitUseRenderers(InsertPtOrError.get(), M,
14211417
DstMIBuilder, Dst, Src);
@@ -1454,25 +1450,26 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
14541450
action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
14551451
const TreePatternNode &Src, const TreePatternNode &Dst, unsigned Start) {
14561452
const CodeGenInstruction *DstI = DstMIBuilder.getCGI();
1457-
const unsigned SrcNumDefs = Src.getExtTypes().size();
1458-
const unsigned DstNumDefs = DstI->Operands.NumDefs;
1459-
if (DstNumDefs == 0)
1460-
return InsertPt;
1461-
1462-
for (unsigned I = Start; I < SrcNumDefs; ++I) {
1463-
std::string OpName = getMangledRootDefName(DstI->Operands[I].Name);
1464-
// CopyRenderer saves a StringRef, so cannot pass OpName itself -
1465-
// let's use a string with an appropriate lifetime.
1466-
StringRef PermanentRef = M.getOperandMatcher(OpName).getSymbolicName();
1467-
DstMIBuilder.addRenderer<CopyRenderer>(PermanentRef);
1468-
}
14691453

14701454
// Some instructions have multiple defs, but are missing a type entry
14711455
// (e.g. s_cc_out operands).
1472-
if (Dst.getExtTypes().size() < DstNumDefs)
1456+
if (Dst.getExtTypes().size() < DstI->Operands.NumDefs)
14731457
return failedImport("unhandled discarded def");
14741458

1475-
for (unsigned I = SrcNumDefs; I < DstNumDefs; ++I) {
1459+
// Process explicit defs. The caller may have already handled the first def.
1460+
for (unsigned I = Start, E = DstI->Operands.NumDefs; I != E; ++I) {
1461+
std::string OpName = getMangledRootDefName(DstI->Operands[I].Name);
1462+
1463+
// If the def is used in the source DAG, forward it.
1464+
if (M.hasOperand(OpName)) {
1465+
// CopyRenderer saves a StringRef, so cannot pass OpName itself -
1466+
// let's use a string with an appropriate lifetime.
1467+
StringRef PermanentRef = M.getOperandMatcher(OpName).getSymbolicName();
1468+
DstMIBuilder.addRenderer<CopyRenderer>(PermanentRef);
1469+
continue;
1470+
}
1471+
1472+
// The def is discarded, create a dead virtual register for it.
14761473
const TypeSetByHwMode &ExtTy = Dst.getExtType(I);
14771474
if (!ExtTy.isMachineValueType())
14781475
return failedImport("unsupported typeset");
@@ -1484,7 +1481,15 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
14841481
unsigned TempRegID = M.allocateTempRegID();
14851482
InsertPt =
14861483
M.insertAction<MakeTempRegisterAction>(InsertPt, *OpTy, TempRegID);
1487-
DstMIBuilder.addRenderer<TempRegRenderer>(TempRegID, true, nullptr, true);
1484+
DstMIBuilder.addRenderer<TempRegRenderer>(
1485+
TempRegID, /*IsDef=*/true, /*SubReg=*/nullptr, /*IsDead=*/true);
1486+
}
1487+
1488+
// Implicit defs are not currently supported, mark all of them as dead.
1489+
for (const Record *Reg : DstI->ImplicitDefs) {
1490+
std::string OpName = getMangledRootDefName(Reg->getName());
1491+
assert(!M.hasOperand(OpName) && "The pattern should've been rejected");
1492+
DstMIBuilder.setDeadImplicitDef(Reg);
14881493
}
14891494

14901495
return InsertPt;
@@ -2299,31 +2304,6 @@ void GlobalISelEmitter::emitRunCustomAction(raw_ostream &OS) {
22992304
<< "}\n";
23002305
}
23012306

2302-
void GlobalISelEmitter::postProcessRule(RuleMatcher &M) {
2303-
SmallPtrSet<const Record *, 16> UsedRegs;
2304-
2305-
// TODO: deal with subregs?
2306-
for (auto &A : M.actions()) {
2307-
auto *MI = dyn_cast<BuildMIAction>(A.get());
2308-
if (!MI)
2309-
continue;
2310-
2311-
for (auto *Use : MI->getCGI()->ImplicitUses)
2312-
UsedRegs.insert(Use);
2313-
}
2314-
2315-
for (auto &A : M.actions()) {
2316-
auto *MI = dyn_cast<BuildMIAction>(A.get());
2317-
if (!MI)
2318-
continue;
2319-
2320-
for (auto *Def : MI->getCGI()->ImplicitDefs) {
2321-
if (!UsedRegs.contains(Def))
2322-
MI->setDeadImplicitDef(Def);
2323-
}
2324-
}
2325-
}
2326-
23272307
void GlobalISelEmitter::run(raw_ostream &OS) {
23282308
if (!UseCoverageFile.empty()) {
23292309
RuleCoverage = CodeGenCoverage();
@@ -2383,7 +2363,6 @@ void GlobalISelEmitter::run(raw_ostream &OS) {
23832363
"Pattern is not covered by a test");
23842364
}
23852365
Rules.push_back(std::move(MatcherOrErr.get()));
2386-
postProcessRule(Rules.back());
23872366
}
23882367

23892368
// Comparison function to order records by name.

0 commit comments

Comments
 (0)