Skip to content

[TableGen][GISel] Improve dead register handling #120426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
define void @read_flags() { ret void }
; CHECK-LABEL: name: read_flags
; CHECK: bb.0:
; CHECK: [[RDFLAGS32_:%[0-9]+]]:gr32 = RDFLAGS32 implicit-def $esp, implicit $esp
; CHECK: [[RDFLAGS32_:%[0-9]+]]:gr32 = RDFLAGS32 implicit-def dead $esp, implicit $esp
; CHECK: $eax = COPY [[RDFLAGS32_]]
...

Expand Down
2 changes: 1 addition & 1 deletion llvm/test/TableGen/GlobalISelEmitter-nested-subregs.td
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ def A0 : RegisterClass<"MyTarget", [i32], 32, (add a0)>;
// CHECK-NEXT: // MIs[0] src
// CHECK-NEXT: GIM_RootCheckType, /*Op*/1, /*Type*/GILLT_s8,
// CHECK-NEXT: // (anyext:{ *:[i16] } i8:{ *:[i8] }:$src) => (EXTRACT_SUBREG:{ *:[i16] } (INSERT_SUBREG:{ *:[i32] } (IMPLICIT_DEF:{ *:[i32] }), A0b:{ *:[i8] }:$src, lo8:{ *:[i32] }), lo16:{ *:[i32] })
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(TargetOpcode::IMPLICIT_DEF),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::INSERT_SUBREG),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/1, /*TempRegID*/1,
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/TableGen/GlobalISelEmitterRegSequence.td
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ def SUBSOME_INSN : I<(outs SRegs:$dst), (ins SOP:$src), []>;
// CHECK-NEXT: GIM_RootCheckType, /*Op*/1, /*Type*/GILLT_s16,
// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/1, /*RC*/GIMT_Encode2(Test::SRegsRegClassID),
// 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] })
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s16,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s16,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(MyTarget::SUBSOME_INSN),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/2, /*OldInsnID*/0, /*OpIdx*/1, // src
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s16,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(MyTarget::SUBSOME_INSN),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/1, // src
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/TableGen/GlobalISelEmitterSubreg.td
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ def : Pat<(sub (complex DOP:$src1, DOP:$src2), 77),
(SOME_INSN2 (EXTRACT_SUBREG DOP:$src1, sub0),
(EXTRACT_SUBREG DOP:$src2, sub1))>;
// 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] }))
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_ComplexSubOperandSubRegRenderer, /*InsnID*/2, /*RendererID*/GIMT_Encode2(0), /*SubOperand*/1, /*SubRegIdx*/GIMT_Encode2(2), // src2
// CHECK-NEXT: GIR_ConstrainOperandRC, /*InsnID*/2, /*Op*/0, GIMT_Encode2(Test::SRegsRegClassID),
// CHECK-NEXT: GIR_ConstrainOperandRC, /*InsnID*/2, /*Op*/1, GIMT_Encode2(Test::DRegsRegClassID),
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_ComplexSubOperandSubRegRenderer, /*InsnID*/1, /*RendererID*/GIMT_Encode2(0), /*SubOperand*/0, /*SubRegIdx*/GIMT_Encode2(1), // src1
Expand Down Expand Up @@ -103,11 +103,11 @@ def : Pat<(i32 (anyext i16:$src)), (INSERT_SUBREG (i32 (IMPLICIT_DEF)), SOP:$src
// instruction.
def : Pat<(i32 (anyext i16:$src)), (SOME_INSN (INSERT_SUBREG (i32 (IMPLICIT_DEF)), SOP:$src, sub0))>;
// CHECK-LABEL: (anyext:{ *:[i32] } i16:{ *:[i16] }:$src) => (SOME_INSN:{ *:[i32] } (INSERT_SUBREG:{ *:[i32] } (IMPLICIT_DEF:{ *:[i32] }), SOP:{ *:[i16] }:$src, sub0:{ *:[i32] }))
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(TargetOpcode::IMPLICIT_DEF),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::INSERT_SUBREG),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/1, /*TempRegID*/1,
Expand Down Expand Up @@ -138,12 +138,12 @@ def : Pat<(i32 (anyext i16:$src)), (INSERT_SUBREG (i32 (COPY_TO_REGCLASS SOP:$sr
// by a subinstruction.
def : Pat<(i32 (anyext i16:$src)), (INSERT_SUBREG (i32 (IMPLICIT_DEF)), (SUBSOME_INSN SOP:$src), sub0)>;
// CHECK-LABEL: (anyext:{ *:[i32] } i16:{ *:[i16] }:$src) => (INSERT_SUBREG:{ *:[i32] } (IMPLICIT_DEF:{ *:[i32] }), (SUBSOME_INSN:{ *:[i16] } SOP:{ *:[i16] }:$src), sub0:{ *:[i32] })
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s16,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(MyTarget::SUBSOME_INSN),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/2, /*OldInsnID*/0, /*OpIdx*/1, // src
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::IMPLICIT_DEF),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/1,
Expand Down Expand Up @@ -200,12 +200,12 @@ def : Pat<(i16 (trunc (bitreverse DOP:$src))),
// CHECK-NEXT: GIM_CheckRegBankForClass, /*MI*/1, /*Op*/1, /*RC*/GIMT_Encode2(Test::DRegsRegClassID),
// CHECK-NEXT: GIM_CheckIsSafeToFold, /*NumInsns*/1,
// CHECK-NEXT: // (trunc:{ *:[i16] } (ctpop:{ *:[i32] } DOP:{ *:[i32] }:$src)) => (SUBSOME_INSN2:{ *:[i16] } (EXTRACT_SUBREG:{ *:[i16] } (SOME_INSN:{ *:[i32] } DOP:{ *:[i32] }:$src), sub0:{ *:[i32] }))
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s16,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/2, /*Opcode*/GIMT_Encode2(MyTarget::SOME_INSN),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/2, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/2, /*OldInsnID*/1, /*OpIdx*/1, // src
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/2,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s16,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
// CHECK-NEXT: GIR_AddTempSubRegister, /*InsnID*/1, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(0), GIMT_Encode2(sub0),
Expand Down
77 changes: 28 additions & 49 deletions llvm/utils/TableGen/GlobalISelEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,6 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
void emitTestSimplePredicate(raw_ostream &OS) override;
void emitRunCustomAction(raw_ostream &OS) override;

void postProcessRule(RuleMatcher &M);

const CodeGenTarget &getTarget() const override { return Target; }
StringRef getClassName() const override { return ClassName; }

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

// Handle additional (ignored) results.
if (DstMIBuilder.getCGI()->Operands.NumDefs > 1) {
InsertPtOrError = importExplicitDefRenderers(
std::prev(*InsertPtOrError), M, DstMIBuilder, Src, Dst, /*Start=*/1);
if (auto Error = InsertPtOrError.takeError())
return std::move(Error);
}
InsertPtOrError = importExplicitDefRenderers(
std::prev(*InsertPtOrError), M, DstMIBuilder, Src, Dst, /*Start=*/1);
if (auto Error = InsertPtOrError.takeError())
return std::move(Error);

InsertPtOrError = importExplicitUseRenderers(InsertPtOrError.get(), M,
DstMIBuilder, Dst, Src);
Expand Down Expand Up @@ -1454,25 +1450,26 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
const TreePatternNode &Src, const TreePatternNode &Dst, unsigned Start) {
const CodeGenInstruction *DstI = DstMIBuilder.getCGI();
const unsigned SrcNumDefs = Src.getExtTypes().size();
const unsigned DstNumDefs = DstI->Operands.NumDefs;
if (DstNumDefs == 0)
return InsertPt;

for (unsigned I = Start; I < SrcNumDefs; ++I) {
std::string OpName = getMangledRootDefName(DstI->Operands[I].Name);
// CopyRenderer saves a StringRef, so cannot pass OpName itself -
// let's use a string with an appropriate lifetime.
StringRef PermanentRef = M.getOperandMatcher(OpName).getSymbolicName();
DstMIBuilder.addRenderer<CopyRenderer>(PermanentRef);
}

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

for (unsigned I = SrcNumDefs; I < DstNumDefs; ++I) {
// Process explicit defs. The caller may have already handled the first def.
for (unsigned I = Start, E = DstI->Operands.NumDefs; I != E; ++I) {
std::string OpName = getMangledRootDefName(DstI->Operands[I].Name);

// If the def is used in the source DAG, forward it.
if (M.hasOperand(OpName)) {
// CopyRenderer saves a StringRef, so cannot pass OpName itself -
// let's use a string with an appropriate lifetime.
StringRef PermanentRef = M.getOperandMatcher(OpName).getSymbolicName();
DstMIBuilder.addRenderer<CopyRenderer>(PermanentRef);
continue;
}

// The def is discarded, create a dead virtual register for it.
const TypeSetByHwMode &ExtTy = Dst.getExtType(I);
if (!ExtTy.isMachineValueType())
return failedImport("unsupported typeset");
Expand All @@ -1484,7 +1481,15 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
unsigned TempRegID = M.allocateTempRegID();
InsertPt =
M.insertAction<MakeTempRegisterAction>(InsertPt, *OpTy, TempRegID);
DstMIBuilder.addRenderer<TempRegRenderer>(TempRegID, true, nullptr, true);
DstMIBuilder.addRenderer<TempRegRenderer>(
TempRegID, /*IsDef=*/true, /*SubReg=*/nullptr, /*IsDead=*/true);
}

// Implicit defs are not currently supported, mark all of them as dead.
for (const Record *Reg : DstI->ImplicitDefs) {
std::string OpName = getMangledRootDefName(Reg->getName());
assert(!M.hasOperand(OpName) && "The pattern should've been rejected");
DstMIBuilder.setDeadImplicitDef(Reg);
}

return InsertPt;
Expand Down Expand Up @@ -2299,31 +2304,6 @@ void GlobalISelEmitter::emitRunCustomAction(raw_ostream &OS) {
<< "}\n";
}

void GlobalISelEmitter::postProcessRule(RuleMatcher &M) {
SmallPtrSet<const Record *, 16> UsedRegs;

// TODO: deal with subregs?
for (auto &A : M.actions()) {
auto *MI = dyn_cast<BuildMIAction>(A.get());
if (!MI)
continue;

for (auto *Use : MI->getCGI()->ImplicitUses)
UsedRegs.insert(Use);
}

for (auto &A : M.actions()) {
auto *MI = dyn_cast<BuildMIAction>(A.get());
if (!MI)
continue;

for (auto *Def : MI->getCGI()->ImplicitDefs) {
if (!UsedRegs.contains(Def))
MI->setDeadImplicitDef(Def);
}
}
}

void GlobalISelEmitter::run(raw_ostream &OS) {
if (!UseCoverageFile.empty()) {
RuleCoverage = CodeGenCoverage();
Expand Down Expand Up @@ -2383,7 +2363,6 @@ void GlobalISelEmitter::run(raw_ostream &OS) {
"Pattern is not covered by a test");
}
Rules.push_back(std::move(MatcherOrErr.get()));
postProcessRule(Rules.back());
}

// Comparison function to order records by name.
Expand Down
Loading