[llvm] [NFC] Add assert to validate `Objects` list for `HwModeSelect` (PR #123794)
Rahul Joshi via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 22 18:59:40 PST 2025
https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/123794
>From f665d25e5814652e2aa17e418b357626c7ef1b38 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 | 16 ++++++++------
llvm/lib/TableGen/Main.cpp | 5 +++++
llvm/lib/TableGen/TGParser.cpp | 11 ++++++----
llvm/test/TableGen/BitsInit.td | 21 +------------------
llvm/test/TableGen/HwModeSelect.td | 5 +++--
llvm/utils/TableGen/Common/CodeGenHwModes.cpp | 8 -------
6 files changed, 26 insertions(+), 40 deletions(-)
diff --git a/llvm/include/llvm/Target/Target.td b/llvm/include/llvm/Target/Target.td
index 3e037affe1cfd2..ebe403c60c2b5e 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,8 @@ 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 +82,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 +93,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 +578,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..ddd4279bf22f48 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -282,15 +282,18 @@ bool TGParser::SetValue(Record *CurRec, SMLoc Loc, const Init *ValName,
if (OverrideDefLoc ? RV->setValue(V, Loc) : RV->setValue(V)) {
std::string InitType;
- if (const auto *BI = dyn_cast<BitsInit>(V))
+ bool LastSingleQuote = true;
+ if (const auto *BI = dyn_cast<BitsInit>(V)) {
InitType = (Twine("' of type bit initializer with length ") +
Twine(BI->getNumBits())).str();
- else if (const auto *TI = dyn_cast<TypedInit>(V))
+ LastSingleQuote = false;
+ } else if (const auto *TI = dyn_cast<TypedInit>(V)) {
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 + (LastSingleQuote ? "'" : ""));
}
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/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);
More information about the llvm-commits
mailing list