Skip to content

Commit 6f72d28

Browse files
authored
[TableGen][GISel] Don't copy dead def from a sub-instruction to the root (#121094)
Sub-instruction can have a def with the same name as a def in a top-level instruction. Previously this could result in both defs copied to the instruction being built.
1 parent cea738b commit 6f72d28

File tree

2 files changed

+38
-9
lines changed

2 files changed

+38
-9
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// RUN: llvm-tblgen -gen-global-isel -optimize-match-table=false \
2+
// RUN: -I %p/../../../include -I %p/../Common %s | FileCheck %s
3+
4+
include "llvm/Target/Target.td"
5+
include "GlobalISelEmitterCommon.td"
6+
7+
// Check that $same_name from I2 isn't copied to the root instruction.
8+
9+
def I1 : I<(outs GPR32:$same_name), (ins GPR32:$rs), []>;
10+
def I2 : I<(outs GPR32:$other_name, GPR32:$same_name), (ins GPR32:$rs), []>;
11+
12+
def : Pat<(abs i32:$x), (I1 (I2 $x))>;
13+
14+
// CHECK-LABEL: // (abs:{ *:[i32] } i32:{ *:[i32] }:$x) => (I1:{ *:[i32] } (I2:{ *:[i32] }:{ *:[i32] } ?:{ *:[i32] }:$x))
15+
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
16+
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
17+
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(MyTarget::I2),
18+
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
19+
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define|RegState::Dead),
20+
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/1, // x
21+
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/1,
22+
// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::I1),
23+
// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[same_name]
24+
// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
25+
// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
26+
// CHECK-NEXT: // GIR_Coverage, 0,
27+
// CHECK-NEXT: GIR_EraseRootFromParent_Done,

llvm/utils/TableGen/GlobalISelEmitter.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -404,9 +404,10 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
404404
createInstructionRenderer(action_iterator InsertPt, RuleMatcher &M,
405405
const TreePatternNode &Dst) const;
406406

407-
Expected<action_iterator> importExplicitDefRenderers(
408-
action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
409-
const TreePatternNode &Dst, unsigned Start = 0) const;
407+
Expected<action_iterator>
408+
importExplicitDefRenderers(action_iterator InsertPt, RuleMatcher &M,
409+
BuildMIAction &DstMIBuilder,
410+
const TreePatternNode &Dst, bool IsRoot) const;
410411

411412
Expected<action_iterator>
412413
importExplicitUseRenderers(action_iterator InsertPt, RuleMatcher &M,
@@ -1369,7 +1370,8 @@ Expected<BuildMIAction &> GlobalISelEmitter::createAndImportInstructionRenderer(
13691370
CopyToPhysRegMIBuilder.addRenderer<CopyPhysRegRenderer>(PhysInput.first);
13701371
}
13711372

1372-
if (auto Error = importExplicitDefRenderers(InsertPt, M, DstMIBuilder, Dst)
1373+
if (auto Error = importExplicitDefRenderers(InsertPt, M, DstMIBuilder, Dst,
1374+
/*IsRoot=*/true)
13731375
.takeError())
13741376
return std::move(Error);
13751377

@@ -1398,8 +1400,8 @@ GlobalISelEmitter::createAndImportSubInstructionRenderer(
13981400
DstMIBuilder.addRenderer<TempRegRenderer>(TempRegID, true);
13991401

14001402
// Handle additional (ignored) results.
1401-
InsertPtOrError = importExplicitDefRenderers(std::prev(*InsertPtOrError), M,
1402-
DstMIBuilder, Dst, /*Start=*/1);
1403+
InsertPtOrError = importExplicitDefRenderers(
1404+
std::prev(*InsertPtOrError), M, DstMIBuilder, Dst, /*IsRoot=*/false);
14031405
if (auto Error = InsertPtOrError.takeError())
14041406
return std::move(Error);
14051407

@@ -1440,16 +1442,16 @@ GlobalISelEmitter::createInstructionRenderer(action_iterator InsertPt,
14401442

14411443
Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
14421444
action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
1443-
const TreePatternNode &Dst, unsigned Start) const {
1445+
const TreePatternNode &Dst, bool IsRoot) const {
14441446
const CodeGenInstruction *DstI = DstMIBuilder.getCGI();
14451447

14461448
// Process explicit defs. The caller may have already handled the first def.
1447-
for (unsigned I = Start, E = DstI->Operands.NumDefs; I != E; ++I) {
1449+
for (unsigned I = IsRoot ? 0 : 1, E = DstI->Operands.NumDefs; I != E; ++I) {
14481450
const CGIOperandList::OperandInfo &OpInfo = DstI->Operands[I];
14491451
std::string OpName = getMangledRootDefName(OpInfo.Name);
14501452

14511453
// If the def is used in the source DAG, forward it.
1452-
if (M.hasOperand(OpName)) {
1454+
if (IsRoot && M.hasOperand(OpName)) {
14531455
// CopyRenderer saves a StringRef, so cannot pass OpName itself -
14541456
// let's use a string with an appropriate lifetime.
14551457
StringRef PermanentRef = M.getOperandMatcher(OpName).getSymbolicName();

0 commit comments

Comments
 (0)