[llvm] [TableGen][NFC] Factor early-out range check. (PR #123645)
Jason Eckhardt via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 20 15:35:52 PST 2025
https://github.com/nvjle updated https://github.com/llvm/llvm-project/pull/123645
>From d919d4d28a2d2343abf68d14ea908d514986ebbf Mon Sep 17 00:00:00 2001
From: Jason Eckhardt <jeckhardt at nvidia.com>
Date: Mon, 20 Jan 2025 10:35:06 -0600
Subject: [PATCH 1/3] [TableGen][NFC] Factor early-out range check.
Combine the EarlyOut and IsContiguous range check.
Also avoid "comparison is always false" warnings in emitted code when
the lower-bound check is against 0.
---
.../utils/TableGen/SearchableTableEmitter.cpp | 41 ++++++++-----------
1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index 38b6f2b3951375..5788e6eb56a683 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -392,37 +392,32 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
}
}
- if (IsContiguous) {
+ if (Index.EarlyOut || IsContiguous) {
const GenericField &Field = Index.Fields[0];
std::string FirstRepr = primaryRepresentation(
Index.Loc, Field, IndexRows[0]->getValueInit(Field.Name));
std::string LastRepr = primaryRepresentation(
Index.Loc, Field, IndexRows.back()->getValueInit(Field.Name));
- OS << " if ((" << Field.Name << " < " << FirstRepr << ") ||\n";
- OS << " (" << Field.Name << " > " << LastRepr << "))\n";
- OS << " return nullptr;\n";
- OS << " auto Table = ArrayRef(" << IndexName << ");\n";
- OS << " size_t Idx = " << Index.Fields[0].Name << " - " << FirstRepr
- << ";\n";
- OS << " return ";
- if (IsPrimary)
- OS << "&Table[Idx]";
+ if (getNumericKey(Index, IndexRows[0]) == 0)
+ OS << " if (";
else
- OS << "&" << Table.Name << "[Table[Idx]._index]";
- OS << ";\n";
- OS << "}\n";
- return;
- }
-
- if (Index.EarlyOut) {
- const GenericField &Field = Index.Fields[0];
- std::string FirstRepr = primaryRepresentation(
- Index.Loc, Field, IndexRows[0]->getValueInit(Field.Name));
- std::string LastRepr = primaryRepresentation(
- Index.Loc, Field, IndexRows.back()->getValueInit(Field.Name));
- OS << " if ((" << Field.Name << " < " << FirstRepr << ") ||\n";
+ OS << " if ((" << Field.Name << " < " << FirstRepr << ") ||\n";
OS << " (" << Field.Name << " > " << LastRepr << "))\n";
OS << " return nullptr;\n\n";
+
+ if (IsContiguous) {
+ OS << " auto Table = ArrayRef(" << IndexName << ");\n";
+ OS << " size_t Idx = " << Index.Fields[0].Name << " - " << FirstRepr
+ << ";\n";
+ OS << " return ";
+ if (IsPrimary)
+ OS << "&Table[Idx]";
+ else
+ OS << "&" << Table.Name << "[Table[Idx]._index]";
+ OS << ";\n";
+ OS << "}\n";
+ return;
+ }
}
OS << " struct KeyType {\n";
>From 5a5e07274b4227b6089d0f50ef87f1ca5ab59790 Mon Sep 17 00:00:00 2001
From: Jason Eckhardt <jeckhardt at nvidia.com>
Date: Mon, 20 Jan 2025 14:38:24 -0600
Subject: [PATCH 2/3] wip
---
.../utils/TableGen/SearchableTableEmitter.cpp | 20 +++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index 5788e6eb56a683..2ee996bffb3813 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -211,20 +211,21 @@ class SearchableTableEmitter {
// known, return that numeric value.
int64_t SearchableTableEmitter::getNumericKey(const SearchIndex &Index,
const Record *Rec) {
- assert(Index.Fields.size() == 1);
+ const GenericField &Field = Index.Fields[0];
// To be consistent with compareBy and primaryRepresentation elsewhere,
// we check for IsInstruction before Enum-- these fields are not exclusive.
- if (Index.Fields[0].IsInstruction) {
- const Record *TheDef = Rec->getValueAsDef(Index.Fields[0].Name);
+ if (Field.IsInstruction) {
+ const Record *TheDef = Rec->getValueAsDef(Field.Name);
return Target->getInstrIntValue(TheDef);
}
- if (Index.Fields[0].Enum) {
- const Record *EnumEntry = Rec->getValueAsDef(Index.Fields[0].Name);
- return Index.Fields[0].Enum->EntryMap[EnumEntry]->second;
+ if (Field.Enum) {
+ const Record *EnumEntry = Rec->getValueAsDef(Field.Name);
+ return Field.Enum->EntryMap[EnumEntry]->second;
}
+ assert(isa<BitsRecTy>(Field.RecType) && "unexpected field type");
- return getInt(Rec, Index.Fields[0].Name);
+ return getInt(Rec, Field.Name);
}
/// Less-than style comparison between \p LHS and \p RHS according to the
@@ -405,10 +406,9 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
OS << " (" << Field.Name << " > " << LastRepr << "))\n";
OS << " return nullptr;\n\n";
- if (IsContiguous) {
+ if (IsContiguous && !Index.EarlyOut) {
OS << " auto Table = ArrayRef(" << IndexName << ");\n";
- OS << " size_t Idx = " << Index.Fields[0].Name << " - " << FirstRepr
- << ";\n";
+ OS << " size_t Idx = " << Field.Name << " - " << FirstRepr << ";\n";
OS << " return ";
if (IsPrimary)
OS << "&Table[Idx]";
>From c28604c0d54f8bd3dbaffc7c40cf1eb46bea606b Mon Sep 17 00:00:00 2001
From: Jason Eckhardt <jeckhardt at nvidia.com>
Date: Mon, 20 Jan 2025 17:33:28 -0600
Subject: [PATCH 3/3] Use clamp, update test accordingly.
---
llvm/test/TableGen/generic-tables-instruction.td | 9 +++------
llvm/test/TableGen/generic-tables.td | 3 +--
llvm/utils/TableGen/SearchableTableEmitter.cpp | 11 ++++++-----
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/llvm/test/TableGen/generic-tables-instruction.td b/llvm/test/TableGen/generic-tables-instruction.td
index 3be2462c9ab69e..7b99784f2d73af 100644
--- a/llvm/test/TableGen/generic-tables-instruction.td
+++ b/llvm/test/TableGen/generic-tables-instruction.td
@@ -18,14 +18,12 @@ def Arch : Target { let InstructionSet = ArchInstrInfo; }
// A contiguous primary (Instruction) key should get a direct lookup instead of
// binary search.
// CHECK: const MyInstr *getCustomEncodingHelper(unsigned Opcode) {
-// CHECK: if ((Opcode < B) ||
-// CHECK: (Opcode > D))
-// CHECK: return nullptr;
+// CHECK: if ((unsigned)Opcode != std::clamp((unsigned)Opcode, (unsigned)B, (unsigned)D))
+// CHECK: return nullptr;
// CHECK: auto Table = ArrayRef(InstrTable);
// CHECK: size_t Idx = Opcode - B;
// CHECK: return &Table[Idx];
-
class MyInstr<int op> : Instruction {
let OutOperandList = (outs);
let InOperandList = (ins);
@@ -56,8 +54,7 @@ def InstrTable : GenericTable {
//
// Verify contiguous check for SearchIndex.
// const MyInfoEntry *getTable2ByValue(uint8_t Value) {
-// CHECK: if ((Value < 0xB) ||
-// CHECK: (Value > 0xD))
+// CHECK: if ((uint8_t)Value != std::clamp((uint8_t)Value, (uint8_t)0xB, (uint8_t)0xD))
// CHECK: return nullptr;
// CHECK: auto Table = ArrayRef(Index);
// CHECK: size_t Idx = Value - 0xB;
diff --git a/llvm/test/TableGen/generic-tables.td b/llvm/test/TableGen/generic-tables.td
index 003b5320934601..8638740a8e565f 100644
--- a/llvm/test/TableGen/generic-tables.td
+++ b/llvm/test/TableGen/generic-tables.td
@@ -113,8 +113,7 @@ def lookupBTableByNameAndFlag : SearchIndex {
// CHECK: const CEntry *lookupCEntry(StringRef Name, unsigned Kind);
// CHECK-LABEL: GET_CTable_IMPL
// CHECK: const CEntry *lookupCEntryByEncoding(uint16_t Encoding) {
-// CHECK: if ((Encoding < 0xA) ||
-// CHECK: (Encoding > 0xF))
+// CHECK: if ((uint16_t)Encoding != std::clamp((uint16_t)Encoding, (uint16_t)0xA, (uint16_t)0xF))
// CHECK: return nullptr;
// CHECK: const CEntry *lookupCEntry(StringRef Name, unsigned Kind) {
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index 2ee996bffb3813..e91baf98e9ffc5 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -211,6 +211,7 @@ class SearchableTableEmitter {
// known, return that numeric value.
int64_t SearchableTableEmitter::getNumericKey(const SearchIndex &Index,
const Record *Rec) {
+ assert(Index.Fields.size() == 1);
const GenericField &Field = Index.Fields[0];
// To be consistent with compareBy and primaryRepresentation elsewhere,
@@ -399,11 +400,11 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
Index.Loc, Field, IndexRows[0]->getValueInit(Field.Name));
std::string LastRepr = primaryRepresentation(
Index.Loc, Field, IndexRows.back()->getValueInit(Field.Name));
- if (getNumericKey(Index, IndexRows[0]) == 0)
- OS << " if (";
- else
- OS << " if ((" << Field.Name << " < " << FirstRepr << ") ||\n";
- OS << " (" << Field.Name << " > " << LastRepr << "))\n";
+ std::string TS =
+ '(' + searchableFieldType(Table, Index, Field, TypeInStaticStruct) +
+ ')';
+ OS << " if (" << TS << Field.Name << " != std::clamp(" << TS << Field.Name
+ << ", " << TS << FirstRepr << ", " << TS << LastRepr << "))\n";
OS << " return nullptr;\n\n";
if (IsContiguous && !Index.EarlyOut) {
More information about the llvm-commits
mailing list