From 4a8b23d6e527b056cddf2f2ec288397f17169cbc Mon Sep 17 00:00:00 2001 From: Sergei Barannikov Date: Wed, 18 Dec 2024 22:20:30 +0300 Subject: [PATCH 1/2] [TableGen][GISel] Learn to import patterns with optional defs --- .../GlobalISelEmitter-optional-def.td | 37 +++++++++++++++++++ .../GlobalISel/GlobalISelMatchTable.cpp | 9 +++-- .../Common/GlobalISel/GlobalISelMatchTable.h | 6 ++- llvm/utils/TableGen/GlobalISelEmitter.cpp | 24 +++++++++--- 4 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 llvm/test/TableGen/GlobalISelEmitter-optional-def.td diff --git a/llvm/test/TableGen/GlobalISelEmitter-optional-def.td b/llvm/test/TableGen/GlobalISelEmitter-optional-def.td new file mode 100644 index 0000000000000..cfb105cd0218b --- /dev/null +++ b/llvm/test/TableGen/GlobalISelEmitter-optional-def.td @@ -0,0 +1,37 @@ +// RUN: llvm-tblgen -gen-global-isel -I %p/../../include -I %p/Common < %s | FileCheck %s + +include "llvm/Target/Target.td" +include "GlobalISelEmitterCommon.td" + +def cc_out : OptionalDefOperand; +def s_cc_out : OptionalDefOperand; + +// CHECK-LABEL: // (add:{ *:[i32] } i32:{ *:[i32] }:$rs1, i32:{ *:[i32] }:$rs2) => (tst2:{ *:[i32] } i32:{ *:[i32] }:$rs1, i32:{ *:[i32] }:$rs2) +// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::tst2), +// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[rd] +// CHECK-NEXT: GIR_AddRegister, /*InsnID*/0, GIMT_Encode2(MyTarget::B0), /*AddRegisterRegFlags*/GIMT_Encode2(RegState::Define | RegState::Dead), +// CHECK-NEXT: GIR_AddRegister, /*InsnID*/0, GIMT_Encode2(MyTarget::F0), /*AddRegisterRegFlags*/GIMT_Encode2(RegState::Define | RegState::Dead), +// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/1, // rs1 +// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/2, // rs2 +// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands, +// CHECK-NEXT: // GIR_Coverage, 1, +// CHECK-NEXT: GIR_EraseRootFromParent_Done, + +// CHECK-LABEL: // (imm:{ *:[i32] }):$imm => (tst1:{ *:[i32] } (imm:{ *:[i32] }):$imm) +// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::tst1), +// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[rd] +// CHECK-NEXT: GIR_AddRegister, /*InsnID*/0, GIMT_Encode2(MyTarget::NoRegister), /*AddRegisterRegFlags*/GIMT_Encode2(RegState::Define | RegState::Dead), +// CHECK-NEXT: GIR_CopyConstantAsSImm, /*NewInsnID*/0, /*OldInsnID*/0, // imm +// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands, +// CHECK-NEXT: // GIR_Coverage, 0, +// CHECK-NEXT: GIR_EraseRootFromParent_Done, + +def tst1 : I<(outs GPR32:$rd, cc_out:$s), (ins i32imm:$imm), + [(set GPR32:$rd, imm:$imm)]>; + +def tst2 : I<(outs GPR32:$rd, s_cc_out:$s), (ins GPR32:$rs1, GPR32:$rs2), + [(set GPR32:$rd, (add i32:$rs1, i32:$rs2))]>; + +// TODO: There should be more tests, but any attempt to write something +// more complex results in tablegen crashing somewhere in +// TreePatternNode::UpdateNodeType. diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp index 15ec7e17130de..619e7a4790c88 100644 --- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp +++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp @@ -1993,10 +1993,13 @@ void AddRegisterRenderer::emitRenderOpcodes(MatchTable &Table, // TODO: This is encoded as a 64-bit element, but only 16 or 32-bits are // really needed for a physical register reference. We can pack the // register and flags in a single field. - if (IsDef) - Table << MatchTable::NamedValue(2, "RegState::Define"); - else + if (IsDef) { + Table << MatchTable::NamedValue( + 2, IsDead ? "RegState::Define | RegState::Dead" : "RegState::Define"); + } else { + assert(!IsDead && "A use cannot be dead"); Table << MatchTable::IntValue(2, 0); + } Table << MatchTable::LineBreak; } diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h index 00fe073057c5c..48ce71be677c0 100644 --- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h +++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h @@ -2091,13 +2091,15 @@ class AddRegisterRenderer : public OperandRenderer { unsigned InsnID; const Record *RegisterDef; bool IsDef; + bool IsDead; const CodeGenTarget &Target; public: AddRegisterRenderer(unsigned InsnID, const CodeGenTarget &Target, - const Record *RegisterDef, bool IsDef = false) + const Record *RegisterDef, bool IsDef = false, + bool IsDead = false) : OperandRenderer(OR_Register), InsnID(InsnID), RegisterDef(RegisterDef), - IsDef(IsDef), Target(Target) {} + IsDef(IsDef), IsDead(IsDead), Target(Target) {} static bool classof(const OperandRenderer *R) { return R->getKind() == OR_Register; diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp index ca05c10ed81ae..ef9bf154767d5 100644 --- a/llvm/utils/TableGen/GlobalISelEmitter.cpp +++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp @@ -1451,14 +1451,10 @@ Expected GlobalISelEmitter::importExplicitDefRenderers( const TreePatternNode &Dst, unsigned Start) { const CodeGenInstruction *DstI = DstMIBuilder.getCGI(); - // Some instructions have multiple defs, but are missing a type entry - // (e.g. s_cc_out operands). - if (Dst.getExtTypes().size() < DstI->Operands.NumDefs) - return failedImport("unhandled discarded def"); - // 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); + const CGIOperandList::OperandInfo &OpInfo = DstI->Operands[I]; + std::string OpName = getMangledRootDefName(OpInfo.Name); // If the def is used in the source DAG, forward it. if (M.hasOperand(OpName)) { @@ -1469,6 +1465,22 @@ Expected GlobalISelEmitter::importExplicitDefRenderers( continue; } + // A discarded explicit def may be an optional physical register. + // If this is the case, add the default register and mark it as dead. + if (OpInfo.Rec->isSubClassOf("OptionalDefOperand")) { + for (const TreePatternNode &DefaultOp : + make_pointee_range(CGP.getDefaultOperand(OpInfo.Rec).DefaultOps)) { + const Record *Reg = cast(DefaultOp.getLeafValue())->getDef(); + + if (!Reg->isSubClassOf("Register") && Reg->getName() != "zero_reg") + return failedImport("optional def is not a register"); + + DstMIBuilder.addRenderer( + Target, Reg, /*IsDef=*/true, /*IsDead=*/true); + } + continue; + } + // The def is discarded, create a dead virtual register for it. const TypeSetByHwMode &ExtTy = Dst.getExtType(I); if (!ExtTy.isMachineValueType()) From 29baa13fd4f569ed68ffc81b317a5d9348c69c75 Mon Sep 17 00:00:00 2001 From: Sergey Barannikov Date: Sat, 21 Dec 2024 04:16:37 +0300 Subject: [PATCH 2/2] Add a few negative tests --- .../GlobalISelEmitter-optional-def.td | 21 ++++++++++++++++++- llvm/utils/TableGen/GlobalISelEmitter.cpp | 9 +++++++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/llvm/test/TableGen/GlobalISelEmitter-optional-def.td b/llvm/test/TableGen/GlobalISelEmitter-optional-def.td index cfb105cd0218b..def4a0447fe53 100644 --- a/llvm/test/TableGen/GlobalISelEmitter-optional-def.td +++ b/llvm/test/TableGen/GlobalISelEmitter-optional-def.td @@ -1,4 +1,6 @@ -// RUN: llvm-tblgen -gen-global-isel -I %p/../../include -I %p/Common < %s | FileCheck %s +// RUN: llvm-tblgen -gen-global-isel -warn-on-skipped-patterns \ +// RUN: -I %p/../../include -I %p/Common %s 2> %t | FileCheck %s +// RUN: FileCheck -DFILE=%s -check-prefix=ERR %s < %t include "llvm/Target/Target.td" include "GlobalISelEmitterCommon.td" @@ -35,3 +37,20 @@ def tst2 : I<(outs GPR32:$rd, s_cc_out:$s), (ins GPR32:$rs1, GPR32:$rs2), // TODO: There should be more tests, but any attempt to write something // more complex results in tablegen crashing somewhere in // TreePatternNode::UpdateNodeType. + + +def not_leaf : OptionalDefOperand; +def not_rec : OptionalDefOperand; +def not_reg : OptionalDefOperand; + +// ERR: [[#@LINE+1]]:5: warning: Skipped pattern: optional def is not a leaf +def tst_not_leaf : I<(outs GPR32:$rd, not_leaf:$s), (ins i32imm:$imm), + [(set GPR32:$rd, imm:$imm)]>; + +// ERR: [[#@LINE+1]]:5: warning: Skipped pattern: optional def is not a record +def tst_not_rec : I<(outs GPR32:$rd, not_rec:$s), (ins i32imm:$imm), + [(set GPR32:$rd, imm:$imm)]>; + +// ERR: [[#@LINE+1]]:5: warning: Skipped pattern: optional def is not a register +def tst_not_reg : I<(outs GPR32:$rd, not_reg:$s), (ins i32imm:$imm), + [(set GPR32:$rd, imm:$imm)]>; diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp index ef9bf154767d5..3f504b73465d2 100644 --- a/llvm/utils/TableGen/GlobalISelEmitter.cpp +++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp @@ -1470,8 +1470,15 @@ Expected GlobalISelEmitter::importExplicitDefRenderers( if (OpInfo.Rec->isSubClassOf("OptionalDefOperand")) { for (const TreePatternNode &DefaultOp : make_pointee_range(CGP.getDefaultOperand(OpInfo.Rec).DefaultOps)) { - const Record *Reg = cast(DefaultOp.getLeafValue())->getDef(); + // TODO: Do these checks in ParseDefaultOperands. + if (!DefaultOp.isLeaf()) + return failedImport("optional def is not a leaf"); + const auto *RegDI = dyn_cast(DefaultOp.getLeafValue()); + if (!RegDI) + return failedImport("optional def is not a record"); + + const Record *Reg = RegDI->getDef(); if (!Reg->isSubClassOf("Register") && Reg->getName() != "zero_reg") return failedImport("optional def is not a register");