Skip to content

[TableGen] Add assert to validate Objects list for HwModeSelect #123794

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 2 commits into from
Jan 27, 2025
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
17 changes: 11 additions & 6 deletions llvm/include/llvm/Target/Target.td
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@ def DefaultMode : HwMode<"", []>;
// "Objects", which is a list of the same length as the list of modes.
// The n-th element on the Objects list will be associated with the n-th
// element on the Modes list.
class HwModeSelect<list<HwMode> Ms> {
class HwModeSelect<list<HwMode> Ms, int ObjectsLength> {
list<HwMode> Modes = Ms;

assert !eq(ObjectsLength, !size(Modes)),
"The Objects and Modes lists must be the same length";
}

// A common class that implements a counterpart of ValueType, which is
Expand All @@ -53,7 +56,7 @@ class HwModeSelect<list<HwMode> Ms> {
// objects could be used. This is specifically applicable to selection
// patterns.
class ValueTypeByHwMode<list<HwMode> Ms, list<ValueType> Ts>
: HwModeSelect<Ms>, ValueType<0, 0> {
: HwModeSelect<Ms, !size(Ts)>, ValueType<0, 0> {
// The length of this list must be the same as the length of Ms.
list<ValueType> Objects = Ts;
}
Expand All @@ -64,7 +67,9 @@ class ValueTypeByHwMode<list<HwMode> Ms, list<ValueType> Ts>
// or ValueType could be used. This is specifically applicable to selection
// patterns.
class PtrValueTypeByHwMode<ValueTypeByHwMode scalar, int addrspace>
: HwModeSelect<scalar.Modes>, PtrValueType<ValueType<0, 0>, addrspace> {
: HwModeSelect<scalar.Modes, !size(scalar.Objects)>,
PtrValueType<ValueType<0, 0>, addrspace> {
// The length of this list must be the same as the length of Ms.
list<ValueType> Objects = scalar.Objects;
}

Expand All @@ -78,7 +83,7 @@ class RegInfo<int RS, int SS, int SA> {

// The register size/alignment information, parameterized by a HW mode.
class RegInfoByHwMode<list<HwMode> Ms = [], list<RegInfo> Ts = []>
: HwModeSelect<Ms> {
: HwModeSelect<Ms, !size(Ts)> {
// The length of this list must be the same as the length of Ms.
list<RegInfo> Objects = Ts;
}
Expand All @@ -89,7 +94,7 @@ class SubRegRange<int size, int offset = 0> {
}

class SubRegRangeByHwMode<list<HwMode> Ms = [], list<SubRegRange> Ts = []>
: HwModeSelect<Ms> {
: HwModeSelect<Ms, !size(Ts)> {
// The length of this list must be the same as the length of Ms.
list<SubRegRange> Objects = Ts;
}
Expand Down Expand Up @@ -574,7 +579,7 @@ class InstructionEncoding {
// an EncodingByHwMode, its Inst and Size members are ignored and Ts are used
// to encode and decode based on HwMode.
class EncodingByHwMode<list<HwMode> Ms = [], list<InstructionEncoding> Ts = []>
: HwModeSelect<Ms> {
: HwModeSelect<Ms, !size(Ts)> {
// The length of this list must be the same as the length of Ms.
list<InstructionEncoding> Objects = Ts;
}
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/TableGen/Main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ int llvm::TableGenMain(const char *argv0,
return 1;
Timer.stopTimer();

// Return early if any other errors were generated during parsing
// (e.g., assert failures).
if (ErrorsPrinted > 0)
return reportError(argv0, Twine(ErrorsPrinted) + " errors.\n");

// Write output to memory.
Timer.startBackendTimer("Backend overall");
std::string OutString;
Expand Down
8 changes: 5 additions & 3 deletions llvm/lib/TableGen/TGParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,13 @@ bool TGParser::SetValue(Record *CurRec, SMLoc Loc, const Init *ValName,
InitType = (Twine("' of type bit initializer with length ") +
Twine(BI->getNumBits())).str();
else if (const auto *TI = dyn_cast<TypedInit>(V))
InitType = (Twine("' of type '") + TI->getType()->getAsString()).str();
InitType =
(Twine("' of type '") + TI->getType()->getAsString() + "'").str();

return Error(Loc, "Field '" + ValName->getAsUnquotedString() +
"' of type '" + RV->getType()->getAsString() +
"' is incompatible with value '" +
V->getAsString() + InitType + "'");
"' is incompatible with value '" + V->getAsString() +
InitType);
}
return false;
}
Expand Down
21 changes: 1 addition & 20 deletions llvm/test/TableGen/BitsInit.td
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@

// RUN: not llvm-tblgen %s 2>&1 > %t
// RUN: FileCheck %s < %t
// RUN: llvm-tblgen %s | FileCheck %s

def a {
bits<2> opc = { 0, 1 };
bits<2> opc2 = { 1, 0 };
bits<1> opc3 = { 1 };
bits<2> a = { opc, opc2 }; // error!
bits<2> b = { opc{0}, opc2{0} };
bits<2> c = { opc{1}, opc2{1} };
bits<2> c = { opc3{0}, opc3 };
Expand All @@ -16,34 +14,25 @@ def a {
// CHECK: bits<2> opc = { 0, 1 };
// CHECK: bits<2> opc2 = { 1, 0 };
// CHECK: bits<1> opc3 = { 1 };
// CHECK: bits<2> a;
// CHECK: bits<2> b = { 1, 0 };
// CHECK: bits<2> c = { 1, 1 };
// CHECK: }

def {
bits<2> B1 = 0b011; // bitfield is too small, reject
bits<3> B2 = 0b011; // ok

bits<2> C1 = 0b111; // bitfield is too small, reject
bits<3> C2 = 0b111; // ok

bits<2> D1 = { 0, 0 }; // ok
bits<2> D2 = { 0b00 }; // ok
bits<3> D3 = { 0, 0 }; // type mismatch. RHS doesn't have enough bits
bits<3> D4 = { 0b00 }; // type mismatch. RHS doesn't have enough bits
bits<1> D5 = { 0 }; // ok
bits<1> D6 = { 1 }; // ok
bits<1> D7 = { 3 }; // type mismatch. LHS doesn't have enough bits
bits<2> D8 = { 0 }; // type mismatch. RHS doesn't have enough bits

bits<8> E;
let E{7...0} = {0,0,1,?,?,?,?,?};
let E{3...0} = 0b0010;

bits<8> F1 = { 0, 1, 0b1001, 0, 0b0 }; // ok
bits<7> F2 = { 0, 1, 0b1001, 0, 0b0 }; // LHS doesn't have enough bits
bits<9> F3 = { 0, 1, 0b1001, 0, 0b0 }; // RHS doesn't have enough bits

bits<8> G1 = { 0, { 1, 0b1001, 0 }, 0b0 }; // ok
bits<8> G2 = { 0, { 1, 0b1001 }, 0, 0b0 }; // ok
Expand All @@ -63,22 +52,14 @@ def {
}

// CHECK: def {{.*}} {
// CHECK: bits<2> B1;
// CHECK: bits<3> B2 = { 0, 1, 1 };
// CHECK: bits<2> C1;
// CHECK: bits<3> C2 = { 1, 1, 1 };
// CHECK: bits<2> D1 = { 0, 0 };
// CHECK: bits<2> D2 = { 0, 0 };
// CHECK: bits<3> D3;
// CHECK: bits<3> D4;
// CHECK: bits<1> D5 = { 0 };
// CHECK: bits<1> D6 = { 1 };
// CHECK: bits<1> D7 = { !cast<bit>(3) };
// CHECK: bits<2> D8;
// CHECK: bits<8> E = { 0, 0, 1, ?, 0, 0, 1, 0 };
// CHECK: bits<8> F1 = { 0, 1, 1, 0, 0, 1, 0, 0 };
// CHECK: bits<7> F2;
// CHECK: bits<9> F3;
// CHECK: bits<8> G1 = { 0, 1, 1, 0, 0, 1, 0, 0 };
// CHECK: bits<8> G2 = { 0, 1, 1, 0, 0, 1, 0, 0 };
// CHECK: bits<8> G3 = { 0, 1, 1, 0, 0, 1, 0, 0 };
Expand Down
37 changes: 37 additions & 0 deletions llvm/test/TableGen/BitsInitErrors.td
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@

// RUN: not llvm-tblgen %s 2>&1 | FileCheck %s -DFILE=%s

def a {
bits<2> opc = { 0, 1 };
bits<2> opc2 = { 1, 0 };
bits<1> opc3 = { 1 };
// CHECK: [[FILE]]:[[@LINE+1]]:15: error: Field 'a' of type 'bits<2>' is incompatible with value '{ opc{1}, opc{0}, opc2{1}, opc2{0} }' of type bit initializer with length 4
bits<2> a = { opc, opc2 }; // error!
}

def {
// CHECK: [[FILE]]:[[@LINE+1]]:16: error: Field 'B1' of type 'bits<2>' is incompatible with value '{ 0, 1, 1 }' of type bit initializer with length 3
bits<2> B1 = 0b011; // bitfield is too small, reject

// CHECK: [[FILE]]:[[@LINE+1]]:16: error: Field 'C1' of type 'bits<2>' is incompatible with value '{ 1, 1, 1 }' of type bit initializer with length 3
bits<2> C1 = 0b111; // bitfield is too small, reject

// CHECK: [[FILE]]:[[@LINE+1]]:16: error: Field 'D3' of type 'bits<3>' is incompatible with value '{ 0, 0 }' of type bit initializer with length 2
bits<3> D3 = { 0, 0 }; // type mismatch. RHS doesn't have enough bits

// CHECK: [[FILE]]:[[@LINE+1]]:16: error: Field 'D4' of type 'bits<3>' is incompatible with value '{ 0, 0 }' of type bit initializer with length 2
bits<3> D4 = { 0b00 }; // type mismatch. RHS doesn't have enough bits

bits<1> D7 = { 3 }; // type mismatch. LHS doesn't have enough bits

// CHECK: [[FILE]]:[[@LINE+1]]:16: error: Field 'D8' of type 'bits<2>' is incompatible with value '{ 0 }' of type bit initializer with length 1
bits<2> D8 = { 0 }; // type mismatch. RHS doesn't have enough bits

// CHECK: [[FILE]]:[[@LINE+1]]:16: error: Field 'F2' of type 'bits<7>' is incompatible with value '{ 0, 1, 1, 0, 0, 1, 0, 0 }' of type bit initializer with length 8
bits<7> F2 = { 0, 1, 0b1001, 0, 0b0 }; // LHS doesn't have enough bits

// CHECK: [[FILE]]:[[@LINE+1]]:16: error: Field 'F3' of type 'bits<9>' is incompatible with value '{ 0, 1, 1, 0, 0, 1, 0, 0 }' of type bit initializer with length 8
bits<9> F3 = { 0, 1, 0b1001, 0, 0b0 }; // RHS doesn't have enough bits

// CHECK: Initializer of 'D7' in 'anonymous_0' could not be fully resolved: { !cast<bit>(3) }
}
5 changes: 3 additions & 2 deletions llvm/test/TableGen/HwModeSelect.td
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: not --crash llvm-tblgen -gen-dag-isel -I %p/../../include %s 2>&1 | FileCheck %s
// RUN: not llvm-tblgen -gen-dag-isel -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s

// The HwModeSelect class is intended to serve as a base class for other
// classes that are then used to select a value based on the HW mode.
Expand All @@ -24,7 +24,8 @@ def HasFeat2 : Predicate<"Subtarget->hasFeat2()">;
def TestMode1 : HwMode<"+feat1", [HasFeat1]>;
def TestMode2 : HwMode<"+feat2", [HasFeat2]>;

// CHECK: error: assertion failed: The Objects and Modes lists must be the same length
// CHECK: [[FILE]]:[[@LINE+1]]:5: error: assertion failed in this record
def BadDef : ValueTypeByHwMode<[TestMode1, TestMode2, DefaultMode],
[i8, i16, i32, i64]>;

// CHECK: error: in record BadDef derived from HwModeSelect: the lists Modes and Objects should have the same size
8 changes: 0 additions & 8 deletions llvm/utils/TableGen/Common/CodeGenHwModes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,6 @@ void HwMode::dump() const { dbgs() << Name << ": " << Features << '\n'; }
HwModeSelect::HwModeSelect(const Record *R, CodeGenHwModes &CGH) {
std::vector<const Record *> Modes = R->getValueAsListOfDefs("Modes");
std::vector<const Record *> Objects = R->getValueAsListOfDefs("Objects");
if (Modes.size() != Objects.size()) {
PrintError(
R->getLoc(),
"in record " + R->getName() +
" derived from HwModeSelect: the lists Modes and Objects should "
"have the same size");
report_fatal_error("error in target description.");
}
for (auto [Mode, Object] : zip_equal(Modes, Objects)) {
unsigned ModeId = CGH.getHwModeId(Mode);
Items.emplace_back(ModeId, Object);
Expand Down
5 changes: 2 additions & 3 deletions mlir/test/mlir-tblgen/attr-or-type-builder-invalid.td
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: not mlir-tblgen -gen-typedef-defs -I %S/../../include %s 2>&1 | FileCheck %s
// RUN: not mlir-tblgen -gen-typedef-defs -I %S/../../include %s 2>&1 | FileCheck %s -DFILE=%s

include "mlir/IR/AttrTypeBase.td"
include "mlir/IR/OpBase.td"
Expand All @@ -13,14 +13,13 @@ class InvalidType<string name> : TypeDef<Test_Dialect, name> {
}

// This definition should not generate an error due to the use in `InvalidTypeA`
// CHECK-NOT: Record `TestParameter' does not have a field named `type'!
def TestParameter : TypeParameter<"int", "int parameter">;

// Test builder uses wrong record class.
// CHECK: [[FILE]]:[[@LINE+1]]:5: error: Initializer of 'typeName' in 'InvalidTypeA' could not be fully resolved: !strconcat("TestDialect", !strconcat(".", mnemonic))
def InvalidTypeA : InvalidType<"InvalidTypeA"> {
let parameters = (ins "int":$v0);
let builders = [
// CHECK: Builder DAG arguments must be either strings or defs which inherit from CArg
TypeBuilder<(ins TestParameter:$arg0), [{
return $_get($_ctxt, arg0);
}]>
Expand Down
Loading