[llvm] [mlir] [TableGen] Add assert to validate `Objects` list for `HwModeSelect` (PR #123794)
Rahul Joshi via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 24 09:38:01 PST 2025
https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/123794
>From 44fa7e9d66e560ade319cdd06475cef2374f4fb9 Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Tue, 21 Jan 2025 10:00:50 -0800
Subject: [PATCH] [NFC] Add assert to validate `Objects` list for HwModeSelect
- Add asserts to validate that the `Objects` and `Modes` lists
for various `HwModeSelect` subclasses are of same length.
---
llvm/include/llvm/Target/Target.td | 17 ++++++---
llvm/lib/TableGen/Main.cpp | 5 +++
llvm/lib/TableGen/TGParser.cpp | 8 ++--
llvm/test/TableGen/BitsInit.td | 21 +----------
llvm/test/TableGen/BitsInitErrors.td | 37 +++++++++++++++++++
llvm/test/TableGen/HwModeSelect.td | 5 ++-
llvm/utils/TableGen/Common/CodeGenHwModes.cpp | 8 ----
.../attr-or-type-builder-invalid.td | 5 +--
8 files changed, 64 insertions(+), 42 deletions(-)
create mode 100644 llvm/test/TableGen/BitsInitErrors.td
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index f568a64971f09e..e8b460aaf803b4 100644
--- a/llvm/include/llvm/Target/Target.td
+++ b/llvm/include/llvm/Target/Target.td
@@ -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
@@ -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;
}
@@ -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;
}
@@ -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;
}
@@ -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;
}
@@ -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;
}
diff --git a/llvm/lib/TableGen/Main.cpp b/llvm/lib/TableGen/Main.cpp
index 55a99cbfc58acd..35600bf2f1f861 100644
--- a/llvm/lib/TableGen/Main.cpp
+++ b/llvm/lib/TableGen/Main.cpp
@@ -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;
diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index d2115ab7627dad..9a8301cffb930b 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -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;
}
diff --git a/llvm/test/TableGen/BitsInit.td b/llvm/test/TableGen/BitsInit.td
index c5527aebb94177..0e7fa7acb722f3 100644
--- a/llvm/test/TableGen/BitsInit.td
+++ b/llvm/test/TableGen/BitsInit.td
@@ -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 };
@@ -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
@@ -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 };
diff --git a/llvm/test/TableGen/BitsInitErrors.td b/llvm/test/TableGen/BitsInitErrors.td
new file mode 100644
index 00000000000000..8e8e8bf179e2c4
--- /dev/null
+++ b/llvm/test/TableGen/BitsInitErrors.td
@@ -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) }
+}
diff --git a/llvm/test/TableGen/HwModeSelect.td b/llvm/test/TableGen/HwModeSelect.td
index e849febe0c4cbf..0bac59a92304de 100644
--- a/llvm/test/TableGen/HwModeSelect.td
+++ b/llvm/test/TableGen/HwModeSelect.td
@@ -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.
@@ -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
diff --git a/llvm/utils/TableGen/Common/CodeGenHwModes.cpp b/llvm/utils/TableGen/Common/CodeGenHwModes.cpp
index c744691ae9e080..9996b5a4451f01 100644
--- a/llvm/utils/TableGen/Common/CodeGenHwModes.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenHwModes.cpp
@@ -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);
diff --git a/mlir/test/mlir-tblgen/attr-or-type-builder-invalid.td b/mlir/test/mlir-tblgen/attr-or-type-builder-invalid.td
index 4db71621845501..d16fcfc4cd0995 100644
--- a/mlir/test/mlir-tblgen/attr-or-type-builder-invalid.td
+++ b/mlir/test/mlir-tblgen/attr-or-type-builder-invalid.td
@@ -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"
@@ -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);
}]>
More information about the llvm-commits
mailing list