[llvm] [TableGen][NFC] Explicitly narrow size_t to unsigned in generated matcher tables (PR #215212)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 23 06:49:54 PDT 2026
https://github.com/gretay-amd updated https://github.com/llvm/llvm-project/pull/215212
>From e43976d71c6456dda655653e7c1c7b2d8e36f910 Mon Sep 17 00:00:00 2001
From: Greta Y <Greta.Yorsh at amd.com>
Date: Thu, 6 Aug 2026 16:14:35 +0100
Subject: [PATCH] [TableGen][NFC] Explicitly narrow size_t to unsigned in
generated matcher tables
This patch handles the following cases:
DAGISelMatcherEmitter emits "unsigned NextRes = Result.size();" in
CheckComplexPattern, where Result is a SmallVectorImpl. Change the type of the
emitted local to size_t. NextRes is only ever used as Result.resize(NextRes+N)
and Result[NextRes+i], both of which take SmallVectorImpl::size_type, so size_t
removes the conversion instead of relocating it and no cast is needed.
InstrInfoEmitter::emitFeatureVerifier and AsmMatcherEmitter::run each emit a
loop over a FeatureBitset, "for (unsigned I = 0, E = MissingFeatures.size();
...)". FeatureBitset::size() returns size_t. Add a static_cast to make the
existing narrowing conversion explicit.
Changing the loop variable to size_t was tried here and is wrong: FeatureBitset
is LLVM's own class, not std::bitset, and its whole indexing interface takes
unsigned (operator[], test, set, reset, find_first_from). size_t would move the
warning from the loop bound to the subscript. unsigned is also what hand-written
LLVM code already uses for this exact loop -- see SystemZAsmParser.cpp,
WebAssemblyAsmParser.cpp and TargetParser/Host.cpp. size() returning size_t is
the outlier; it returns the compile-time constant MAX_SUBTARGET_FEATURES, so the
conversion is NFC.
AsmMatcherEmitter::emitCustomOperandParsing emits "unsigned NextOpNum =
Operands.size() - 1;", where Operands is an OperandVector. Add a static_cast to
make the existing narrowing conversion explicit. The cast has to wrap the whole
expression because the "- 1" is appended conditionally on HasMnemonicFirst.
NextOpNum is used only as a bit position in "(1 << NextOpNum)" against a
32-bit OperandMask, so unsigned is the correct type and the value is bounded by
the operand count of a single parsed instruction.
GlobalISelCombinerEmitter emits setRuleEnabled and setRuleDisabled, which walk a
rule-index range of type std::pair<uint64_t, uint64_t> and pass each index to
SparseBitVector::set and ::reset. Those take unsigned, so the value narrowed at
the call. Add a static_cast at the call site. A combine rule index is an index
into AllCombineRules and the emitted class already declares
isRuleEnabled(unsigned RuleID), so unsigned is the correct destination type and
the conversion is NFC.
Narrowing the emitted helpers instead -- changing getRuleIdxForIdentifier and
getRuleRangeForIdentifier to return unsigned and declaring the emitted local
unsigned -- was tried and rejected on two grounds. It does not remove the
conversion, it relocates it into StringRef::getAsInteger<unsigned>, which then
warns inside llvm/include/llvm/ADT/StringRef.h in every translation unit that
includes a combiner table. And it is not NFC: getAsInteger reports failure on
overflow, so an out-of-range numeric argument to -<pass>-disable-rule would stop
parsing rather than being truncated at the SparseBitVector call.
This patch changes the string literals in three TableGen backends and the
generated code of every target. It fixes 6 instances of MSVC warning C4244 and
4 of C4267 ("possible loss of data") in the AMDGPU tables, across
AMDGPUGenDAGISel.inc, AMDGPUGenInstrInfo.inc, AMDGPUGenAsmMatcher.inc,
AMDGPUGenPreLegalizeGICombiner.inc, AMDGPUGenPostLegalizeGICombiner.inc and
AMDGPUGenRegBankGICombiner.inc. Five source lines produce ten warnings because
the combiner emitter line runs twice (for (bool Enabled : {true, false})) and
lands in three combiner tables.
Because these are shared emitters the same fixes clear the corresponding
warnings in other targets' tables: in a build configured for X86 and AMDGPU they
remove 4 further instances of C4244 and 5 of C4267 in the X86 and R600 tables.
The C4244 left in X86GenDAGISel.inc come from X86's own .td predicate bodies and
are not addressed here.
Assisted-by: Claude <noreply at anthropic.com>
---
llvm/utils/TableGen/AsmMatcherEmitter.cpp | 8 ++++----
llvm/utils/TableGen/DAGISelMatcherEmitter.cpp | 2 +-
llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp | 3 ++-
llvm/utils/TableGen/InstrInfoEmitter.cpp | 4 +++-
4 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/llvm/utils/TableGen/AsmMatcherEmitter.cpp b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
index b756582603ffdf..7ebffdb19469dd 100644
--- a/llvm/utils/TableGen/AsmMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/AsmMatcherEmitter.cpp
@@ -3269,8 +3269,8 @@ emitCustomOperandParsing(raw_ostream &OS, CodeGenTarget &Target,
"getAvailableFeatures();\n\n";
OS << " // Get the next operand index.\n";
- OS << " unsigned NextOpNum = Operands.size()"
- << (HasMnemonicFirst ? " - 1" : "") << ";\n";
+ OS << " unsigned NextOpNum = static_cast<unsigned>(Operands.size()"
+ << (HasMnemonicFirst ? " - 1" : "") << ");\n";
// Emit code to search the table.
OS << " // Search the table.\n";
@@ -3526,8 +3526,8 @@ static void emitFeatureCheck(raw_ostream &OS, bool ReportMultipleNearMisses) {
"~AvailableFeatures;\n";
OS << " DEBUG_WITH_TYPE(\"asm-matcher\", dbgs() << \"Missing target "
"features:\";\n";
- OS << " for (unsigned I = 0, E = "
- "NewMissingFeatures.size(); I != E; ++I)\n";
+ OS << " for (unsigned I = 0, E = static_cast<unsigned>("
+ "NewMissingFeatures.size()); I != E; ++I)\n";
OS << " if (NewMissingFeatures[I])\n";
OS << " dbgs() << ' ' << I;\n";
OS << " dbgs() << \"\\n\");\n";
diff --git a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
index 37e7ad34cb2252..6a4d4823a23354 100644
--- a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
@@ -1380,7 +1380,7 @@ void MatcherTableEmitter::EmitPredicateFunctions(raw_ostream &OS) {
" SmallVectorImpl<std::pair<SDValue, SDNode *>> &Result)",
true /*AddOverride*/);
OS << "{\n";
- OS << " unsigned NextRes = Result.size();\n";
+ OS << " size_t NextRes = Result.size();\n";
OS << " switch (PatternNo) {\n";
OS << " default: llvm_unreachable(\"Invalid pattern # in table?\");\n";
for (unsigned i = 0, e = ComplexPatterns.size(); i != e; ++i) {
diff --git a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
index 60a3047f4a0298..a9e9a81326dd02 100644
--- a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
@@ -2525,7 +2525,8 @@ void GICombinerEmitter::emitRuleConfigImpl(raw_ostream &OS) {
<< " if (!MaybeRange)\n"
<< " return false;\n"
<< " for (auto I = MaybeRange->first; I < MaybeRange->second; ++I)\n"
- << " DisabledRules." << (Enabled ? "reset" : "set") << "(I);\n"
+ << " DisabledRules." << (Enabled ? "reset" : "set")
+ << "(static_cast<unsigned>(I));\n"
<< " return true;\n"
<< "}\n\n";
}
diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp
index 2bdde2df38751f..a398c470a7aa81 100644
--- a/llvm/utils/TableGen/InstrInfoEmitter.cpp
+++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp
@@ -854,7 +854,9 @@ void InstrInfoEmitter::emitFeatureVerifier(raw_ostream &OS,
<< " Msg << \"Attempting to emit \" << &" << Target.getName()
<< "InstrNameData[" << Target.getName() << "InstrNameIndices[Opcode]]\n"
<< " << \" instruction but the \";\n"
- << " for (unsigned i = 0, e = MissingFeatures.size(); i != e; ++i)\n"
+ << " for (unsigned i = 0, e = "
+ "static_cast<unsigned>(MissingFeatures.size());\n"
+ << " i != e; ++i)\n"
<< " if (MissingFeatures.test(i))\n"
<< " Msg << SubtargetFeatureNames[i] << \" \";\n"
<< " Msg << \"predicate(s) are not met\";\n"
More information about the llvm-commits
mailing list