[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch intrinsics to StringTable and work around MSVC (PR #123548)
Chandler Carruth via lldb-commits
lldb-commits at lists.llvm.org
Sun Jan 19 21:04:25 PST 2025
https://github.com/chandlerc created https://github.com/llvm/llvm-project/pull/123548
**Note:** This PR depends on #123302 and #123308 -- only the last of the three commits should be reviewed here.
---
Historically, the main example of *very* large string tables used the
`EmitCharArray` to work around MSVC limitations with string literals,
but that was switched (without removing the API) in order to consolidate
on a nicer emission primitive.
While this large string table in `IntrinsicsImpl.inc` seems to compile
correctly on MSVC without the work around in `EmitCharArray` (and that
this PR adds back to the nicer emission path), other users have
repeatedly hit this MSVC limitation as you can see in the discussion on
PR https://github.com/llvm/llvm-project/pull/120534. This PR teaches the string offset table emission to look at
the size of the table and switch to the char array emission strategy
when the table becomes too large.
This work around does have the downside of making compile times worse
for large string tables, but that appears unavoidable until we can
identify known good MSVC versions and switch to requiring them for all
LLVM users. It also reduces searchability of the generated string table
-- I looked at emitting a comment with each string but it is tricky
because the escaping rules for an inline comment are different from
those of of a string literal, and there's no real way to turn the string
literal into a comment.
This PR also switches the `IntrinsicsImpl.inc` string tables over to the
new `StringTable` runtime abstraction. I didn't want to do this until
landing the MSVC workaround in case it caused even this example to start
hitting the MSVC bug, but I wanted to switch here so that I could
simplify the API for emitting the string table with the workaround
present. With the two different emission strategies, its important to
use a very exact syntax and that seems better encapsulated in the API.
This PR should unblock landing https://github.com/llvm/llvm-project/pull/120534 and letting us switch all of
Clang's builtins to use string tables. That PR has all the details
motivating the overall effort.
Follow-up patches will try to consolidate the remaining users onto the
single interface, but those at least were easy to separate into
follow-ups and keep this PR somewhat smaller.
>From 9a525fa4322d6a46154305097185ec017916d01f Mon Sep 17 00:00:00 2001
From: Chandler Carruth <chandlerc at gmail.com>
Date: Fri, 17 Jan 2025 08:50:44 +0000
Subject: [PATCH 1/3] [StrTable] Switch the option parser to
`llvm::StringTable`
Now that we have a dedicated abstraction for string tables, switch the
option parser library's string table over to it rather than using a raw
`const char*`. Also try to use the `StringTable::Offset` type rather
than a raw `unsigned` where we can to avoid accidental increments or
other issues.
This is based on review feedback for the initial switch of options to
a string table. Happy to tweak or adjust if desired here.
---
clang/lib/Frontend/CompilerInvocation.cpp | 2 +-
.../Platform/MacOSX/PlatformDarwin.cpp | 3 +-
llvm/include/llvm/Option/OptTable.h | 67 ++++++++++--------
llvm/lib/Option/OptTable.cpp | 70 ++++++++++---------
llvm/tools/llvm-objdump/llvm-objdump.cpp | 3 +-
.../Option/OptionMarshallingTest.cpp | 3 +-
llvm/utils/TableGen/OptionParserEmitter.cpp | 11 +--
7 files changed, 88 insertions(+), 71 deletions(-)
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 58658dedbaf1ee..3bf124e4827be9 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -282,7 +282,7 @@ using ArgumentConsumer = CompilerInvocation::ArgumentConsumer;
#undef OPTTABLE_STR_TABLE_CODE
static llvm::StringRef lookupStrInTable(unsigned Offset) {
- return &OptionStrTable[Offset];
+ return OptionStrTable[Offset];
}
#define SIMPLE_ENUM_VALUE_TABLE
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 2a36f95c94d0ce..51e9a6d81b8390 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -42,6 +42,7 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Timer.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringTable.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Threading.h"
@@ -1083,7 +1084,7 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
if (!version.empty() && sdk_type != XcodeSDK::Type::Linux &&
sdk_type != XcodeSDK::Type::XROS) {
#define OPTION(PREFIX_OFFSET, NAME_OFFSET, VAR, ...) \
- llvm::StringRef opt_##VAR = &OptionStrTable[NAME_OFFSET]; \
+ llvm::StringRef opt_##VAR = OptionStrTable[NAME_OFFSET]; \
(void)opt_##VAR;
#include "clang/Driver/Options.inc"
#undef OPTION
diff --git a/llvm/include/llvm/Option/OptTable.h b/llvm/include/llvm/Option/OptTable.h
index 38a03fef7ae124..61a58aa304ecb4 100644
--- a/llvm/include/llvm/Option/OptTable.h
+++ b/llvm/include/llvm/Option/OptTable.h
@@ -12,6 +12,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringTable.h"
#include "llvm/Option/OptSpecifier.h"
#include "llvm/Support/StringSaver.h"
#include <cassert>
@@ -54,7 +55,7 @@ class OptTable {
/// Entry for a single option instance in the option data table.
struct Info {
unsigned PrefixesOffset;
- unsigned PrefixedNameOffset;
+ StringTable::Offset PrefixedNameOffset;
const char *HelpText;
// Help text for specific visibilities. A list of pairs, where each pair
// is a list of visibilities and a specific help string for those
@@ -80,34 +81,37 @@ class OptTable {
bool hasNoPrefix() const { return PrefixesOffset == 0; }
- unsigned getNumPrefixes(ArrayRef<unsigned> PrefixesTable) const {
- return PrefixesTable[PrefixesOffset];
+ unsigned getNumPrefixes(ArrayRef<StringTable::Offset> PrefixesTable) const {
+ // We embed the number of prefixes in the value of the first offset.
+ return PrefixesTable[PrefixesOffset].value();
}
- ArrayRef<unsigned>
- getPrefixOffsets(ArrayRef<unsigned> PrefixesTable) const {
- return hasNoPrefix() ? ArrayRef<unsigned>()
+ ArrayRef<StringTable::Offset>
+ getPrefixOffsets(ArrayRef<StringTable::Offset> PrefixesTable) const {
+ return hasNoPrefix() ? ArrayRef<StringTable::Offset>()
: PrefixesTable.slice(PrefixesOffset + 1,
getNumPrefixes(PrefixesTable));
}
- void appendPrefixes(const char *StrTable, ArrayRef<unsigned> PrefixesTable,
+ void appendPrefixes(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
SmallVectorImpl<StringRef> &Prefixes) const {
- for (unsigned PrefixOffset : getPrefixOffsets(PrefixesTable))
- Prefixes.push_back(&StrTable[PrefixOffset]);
+ for (auto PrefixOffset : getPrefixOffsets(PrefixesTable))
+ Prefixes.push_back(StrTable[PrefixOffset]);
}
- StringRef getPrefix(const char *StrTable, ArrayRef<unsigned> PrefixesTable,
+ StringRef getPrefix(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
unsigned PrefixIndex) const {
- return &StrTable[getPrefixOffsets(PrefixesTable)[PrefixIndex]];
+ return StrTable[getPrefixOffsets(PrefixesTable)[PrefixIndex]];
}
- StringRef getPrefixedName(const char *StrTable) const {
- return &StrTable[PrefixedNameOffset];
+ StringRef getPrefixedName(const StringTable &StrTable) const {
+ return StrTable[PrefixedNameOffset];
}
- StringRef getName(const char *StrTable,
- ArrayRef<unsigned> PrefixesTable) const {
+ StringRef getName(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable) const {
unsigned PrefixLength =
hasNoPrefix() ? 0 : getPrefix(StrTable, PrefixesTable, 0).size();
return getPrefixedName(StrTable).drop_front(PrefixLength);
@@ -117,13 +121,13 @@ class OptTable {
private:
// A unified string table for these options. Individual strings are stored as
// null terminated C-strings at offsets within this table.
- const char *StrTable;
+ const StringTable *StrTable;
// A table of different sets of prefixes. Each set starts with the number of
// prefixes in that set followed by that many offsets into the string table
// for each of the prefix strings. This is essentially a Pascal-string style
// encoding.
- ArrayRef<unsigned> PrefixesTable;
+ ArrayRef<StringTable::Offset> PrefixesTable;
/// The option information table.
ArrayRef<Info> OptionInfos;
@@ -161,7 +165,8 @@ class OptTable {
protected:
/// Initialize OptTable using Tablegen'ed OptionInfos. Child class must
/// manually call \c buildPrefixChars once they are fully constructed.
- OptTable(const char *StrTable, ArrayRef<unsigned> PrefixesTable,
+ OptTable(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos, bool IgnoreCase = false);
/// Build (or rebuild) the PrefixChars member.
@@ -171,10 +176,12 @@ class OptTable {
virtual ~OptTable();
/// Return the string table used for option names.
- const char *getStrTable() const { return StrTable; }
+ const StringTable &getStrTable() const { return *StrTable; }
/// Return the prefixes table used for option names.
- ArrayRef<unsigned> getPrefixesTable() const { return PrefixesTable; }
+ ArrayRef<StringTable::Offset> getPrefixesTable() const {
+ return PrefixesTable;
+ }
/// Return the total number of option classes.
unsigned getNumOptions() const { return OptionInfos.size(); }
@@ -187,25 +194,25 @@ class OptTable {
/// Lookup the name of the given option.
StringRef getOptionName(OptSpecifier id) const {
- return getInfo(id).getName(StrTable, PrefixesTable);
+ return getInfo(id).getName(*StrTable, PrefixesTable);
}
/// Lookup the prefix of the given option.
StringRef getOptionPrefix(OptSpecifier id) const {
const Info &I = getInfo(id);
return I.hasNoPrefix() ? StringRef()
- : I.getPrefix(StrTable, PrefixesTable, 0);
+ : I.getPrefix(*StrTable, PrefixesTable, 0);
}
void appendOptionPrefixes(OptSpecifier id,
SmallVectorImpl<StringRef> &Prefixes) const {
const Info &I = getInfo(id);
- I.appendPrefixes(StrTable, PrefixesTable, Prefixes);
+ I.appendPrefixes(*StrTable, PrefixesTable, Prefixes);
}
/// Lookup the prefixed name of the given option.
StringRef getOptionPrefixedName(OptSpecifier id) const {
- return getInfo(id).getPrefixedName(StrTable);
+ return getInfo(id).getPrefixedName(*StrTable);
}
/// Get the kind of the given option.
@@ -418,19 +425,21 @@ class OptTable {
/// Specialization of OptTable
class GenericOptTable : public OptTable {
protected:
- GenericOptTable(const char *StrTable, ArrayRef<unsigned> PrefixesTable,
+ GenericOptTable(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos, bool IgnoreCase = false);
};
class PrecomputedOptTable : public OptTable {
protected:
- PrecomputedOptTable(const char *StrTable, ArrayRef<unsigned> PrefixesTable,
+ PrecomputedOptTable(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos,
- ArrayRef<unsigned> PrefixesUnionOffsets,
+ ArrayRef<StringTable::Offset> PrefixesUnionOffsets,
bool IgnoreCase = false)
: OptTable(StrTable, PrefixesTable, OptionInfos, IgnoreCase) {
- for (unsigned PrefixOffset : PrefixesUnionOffsets)
- PrefixesUnion.push_back(&StrTable[PrefixOffset]);
+ for (auto PrefixOffset : PrefixesUnionOffsets)
+ PrefixesUnion.push_back(StrTable[PrefixOffset]);
buildPrefixChars();
}
};
diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index 87e6f1f12364c2..6d10e6154147ec 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -33,11 +33,12 @@ using namespace llvm::opt;
namespace {
struct OptNameLess {
- const char *StrTable;
- ArrayRef<unsigned> PrefixesTable;
+ const StringTable *StrTable;
+ ArrayRef<StringTable::Offset> PrefixesTable;
- explicit OptNameLess(const char *StrTable, ArrayRef<unsigned> PrefixesTable)
- : StrTable(StrTable), PrefixesTable(PrefixesTable) {}
+ explicit OptNameLess(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable)
+ : StrTable(&StrTable), PrefixesTable(PrefixesTable) {}
#ifndef NDEBUG
inline bool operator()(const OptTable::Info &A,
@@ -45,13 +46,13 @@ struct OptNameLess {
if (&A == &B)
return false;
- if (int Cmp = StrCmpOptionName(A.getName(StrTable, PrefixesTable),
- B.getName(StrTable, PrefixesTable)))
+ if (int Cmp = StrCmpOptionName(A.getName(*StrTable, PrefixesTable),
+ B.getName(*StrTable, PrefixesTable)))
return Cmp < 0;
SmallVector<StringRef, 8> APrefixes, BPrefixes;
- A.appendPrefixes(StrTable, PrefixesTable, APrefixes);
- B.appendPrefixes(StrTable, PrefixesTable, BPrefixes);
+ A.appendPrefixes(*StrTable, PrefixesTable, APrefixes);
+ B.appendPrefixes(*StrTable, PrefixesTable, BPrefixes);
if (int Cmp = StrCmpOptionPrefixes(APrefixes, BPrefixes))
return Cmp < 0;
@@ -68,7 +69,7 @@ struct OptNameLess {
// Support lower_bound between info and an option name.
inline bool operator()(const OptTable::Info &I, StringRef Name) const {
// Do not fallback to case sensitive comparison.
- return StrCmpOptionName(I.getName(StrTable, PrefixesTable), Name, false) <
+ return StrCmpOptionName(I.getName(*StrTable, PrefixesTable), Name, false) <
0;
}
};
@@ -76,9 +77,10 @@ struct OptNameLess {
OptSpecifier::OptSpecifier(const Option *Opt) : ID(Opt->getID()) {}
-OptTable::OptTable(const char *StrTable, ArrayRef<unsigned> PrefixesTable,
+OptTable::OptTable(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos, bool IgnoreCase)
- : StrTable(StrTable), PrefixesTable(PrefixesTable),
+ : StrTable(&StrTable), PrefixesTable(PrefixesTable),
OptionInfos(OptionInfos), IgnoreCase(IgnoreCase) {
// Explicitly zero initialize the error to work around a bug in array
// value-initialization on MinGW with gcc 4.3.5.
@@ -151,13 +153,13 @@ static bool isInput(const ArrayRef<StringRef> &Prefixes, StringRef Arg) {
}
/// \returns Matched size. 0 means no match.
-static unsigned matchOption(const char *StrTable,
- ArrayRef<unsigned> PrefixesTable,
+static unsigned matchOption(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
const OptTable::Info *I, StringRef Str,
bool IgnoreCase) {
StringRef Name = I->getName(StrTable, PrefixesTable);
- for (unsigned PrefixOffset : I->getPrefixOffsets(PrefixesTable)) {
- StringRef Prefix = &StrTable[PrefixOffset];
+ for (auto PrefixOffset : I->getPrefixOffsets(PrefixesTable)) {
+ StringRef Prefix = StrTable[PrefixOffset];
if (Str.starts_with(Prefix)) {
StringRef Rest = Str.substr(Prefix.size());
bool Matched = IgnoreCase ? Rest.starts_with_insensitive(Name)
@@ -170,13 +172,13 @@ static unsigned matchOption(const char *StrTable,
}
// Returns true if one of the Prefixes + In.Names matches Option
-static bool optionMatches(const char *StrTable,
- ArrayRef<unsigned> PrefixesTable,
+static bool optionMatches(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
const OptTable::Info &In, StringRef Option) {
StringRef Name = In.getName(StrTable, PrefixesTable);
if (Option.consume_back(Name))
- for (unsigned PrefixOffset : In.getPrefixOffsets(PrefixesTable))
- if (Option == &StrTable[PrefixOffset])
+ for (auto PrefixOffset : In.getPrefixOffsets(PrefixesTable))
+ if (Option == StrTable[PrefixOffset])
return true;
return false;
}
@@ -189,7 +191,7 @@ OptTable::suggestValueCompletions(StringRef Option, StringRef Arg) const {
// Search all options and return possible values.
for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) {
const Info &In = OptionInfos[I];
- if (!In.Values || !optionMatches(StrTable, PrefixesTable, In, Option))
+ if (!In.Values || !optionMatches(*StrTable, PrefixesTable, In, Option))
continue;
SmallVector<StringRef, 8> Candidates;
@@ -217,9 +219,9 @@ OptTable::findByPrefix(StringRef Cur, Visibility VisibilityMask,
if (In.Flags & DisableFlags)
continue;
- StringRef Name = In.getName(StrTable, PrefixesTable);
- for (unsigned PrefixOffset : In.getPrefixOffsets(PrefixesTable)) {
- StringRef Prefix = &StrTable[PrefixOffset];
+ StringRef Name = In.getName(*StrTable, PrefixesTable);
+ for (auto PrefixOffset : In.getPrefixOffsets(PrefixesTable)) {
+ StringRef Prefix = (*StrTable)[PrefixOffset];
std::string S = (Twine(Prefix) + Name + "\t").str();
if (In.HelpText)
S += In.HelpText;
@@ -271,7 +273,7 @@ unsigned OptTable::internalFindNearest(
for (const Info &CandidateInfo :
ArrayRef<Info>(OptionInfos).drop_front(FirstSearchableIndex)) {
- StringRef CandidateName = CandidateInfo.getName(StrTable, PrefixesTable);
+ StringRef CandidateName = CandidateInfo.getName(*StrTable, PrefixesTable);
// We can eliminate some option prefix/name pairs as candidates right away:
// * Ignore option candidates with empty names, such as "--", or names
@@ -304,9 +306,9 @@ unsigned OptTable::internalFindNearest(
// Consider each possible prefix for each candidate to find the most
// appropriate one. For example, if a user asks for "--helm", suggest
// "--help" over "-help".
- for (unsigned CandidatePrefixOffset :
+ for (auto CandidatePrefixOffset :
CandidateInfo.getPrefixOffsets(PrefixesTable)) {
- StringRef CandidatePrefix = &StrTable[CandidatePrefixOffset];
+ StringRef CandidatePrefix = (*StrTable)[CandidatePrefixOffset];
// If Candidate and NormalizedName have more than 'BestDistance'
// characters of difference, no need to compute the edit distance, it's
// going to be greater than BestDistance. Don't bother computing Candidate
@@ -359,14 +361,14 @@ std::unique_ptr<Arg> OptTable::parseOneArgGrouped(InputArgList &Args,
StringRef Name = Str.ltrim(PrefixChars);
const Info *Start =
std::lower_bound(OptionInfos.data() + FirstSearchableIndex, End, Name,
- OptNameLess(StrTable, PrefixesTable));
+ OptNameLess(*StrTable, PrefixesTable));
const Info *Fallback = nullptr;
unsigned Prev = Index;
// Search for the option which matches Str.
for (; Start != End; ++Start) {
unsigned ArgSize =
- matchOption(StrTable, PrefixesTable, Start, Str, IgnoreCase);
+ matchOption(*StrTable, PrefixesTable, Start, Str, IgnoreCase);
if (!ArgSize)
continue;
@@ -449,7 +451,7 @@ std::unique_ptr<Arg> OptTable::internalParseOneArg(
// Search for the first next option which could be a prefix.
Start =
- std::lower_bound(Start, End, Name, OptNameLess(StrTable, PrefixesTable));
+ std::lower_bound(Start, End, Name, OptNameLess(*StrTable, PrefixesTable));
// Options are stored in sorted order, with '\0' at the end of the
// alphabet. Since the only options which can accept a string must
@@ -464,7 +466,7 @@ std::unique_ptr<Arg> OptTable::internalParseOneArg(
// Scan for first option which is a proper prefix.
for (; Start != End; ++Start)
if ((ArgSize =
- matchOption(StrTable, PrefixesTable, Start, Str, IgnoreCase)))
+ matchOption(*StrTable, PrefixesTable, Start, Str, IgnoreCase)))
break;
if (Start == End)
break;
@@ -787,15 +789,15 @@ void OptTable::internalPrintHelp(
OS.flush();
}
-GenericOptTable::GenericOptTable(const char *StrTable,
- ArrayRef<unsigned> PrefixesTable,
+GenericOptTable::GenericOptTable(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos, bool IgnoreCase)
: OptTable(StrTable, PrefixesTable, OptionInfos, IgnoreCase) {
std::set<StringRef> TmpPrefixesUnion;
for (auto const &Info : OptionInfos.drop_front(FirstSearchableIndex))
- for (unsigned PrefixOffset : Info.getPrefixOffsets(PrefixesTable))
- TmpPrefixesUnion.insert(StringRef(&StrTable[PrefixOffset]));
+ for (auto PrefixOffset : Info.getPrefixOffsets(PrefixesTable))
+ TmpPrefixesUnion.insert(StrTable[PrefixOffset]);
PrefixesUnion.append(TmpPrefixesUnion.begin(), TmpPrefixesUnion.end());
buildPrefixChars();
}
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 93fed8ee8e6f42..99e0440dce78d5 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -94,7 +94,8 @@ namespace {
class CommonOptTable : public opt::GenericOptTable {
public:
- CommonOptTable(const char *StrTable, ArrayRef<unsigned> PrefixesTable,
+ CommonOptTable(const StringTable &StrTable,
+ ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos, const char *Usage,
const char *Description)
: opt::GenericOptTable(StrTable, PrefixesTable, OptionInfos),
diff --git a/llvm/unittests/Option/OptionMarshallingTest.cpp b/llvm/unittests/Option/OptionMarshallingTest.cpp
index 08c3b019689f8c..005144b91bf7f3 100644
--- a/llvm/unittests/Option/OptionMarshallingTest.cpp
+++ b/llvm/unittests/Option/OptionMarshallingTest.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringTable.h"
#include "gtest/gtest.h"
#define OPTTABLE_STR_TABLE_CODE
@@ -20,7 +21,7 @@ struct OptionWithMarshallingInfo {
const char *ImpliedValue;
llvm::StringRef getPrefixedName() const {
- return &OptionStrTable[PrefixedNameOffset];
+ return OptionStrTable[PrefixedNameOffset];
}
};
diff --git a/llvm/utils/TableGen/OptionParserEmitter.cpp b/llvm/utils/TableGen/OptionParserEmitter.cpp
index 8b92d252392194..35a452890b0ec7 100644
--- a/llvm/utils/TableGen/OptionParserEmitter.cpp
+++ b/llvm/utils/TableGen/OptionParserEmitter.cpp
@@ -303,15 +303,17 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) {
OS << "/////////\n";
OS << "// String table\n\n";
OS << "#ifdef OPTTABLE_STR_TABLE_CODE\n";
- Table.EmitStringLiteralDef(OS, "static constexpr char OptionStrTable[]",
- /*Indent=*/"");
+ Table.EmitStringLiteralDef(
+ OS, "static constexpr llvm::StringTable OptionStrTable",
+ /*Indent=*/"");
OS << "#endif // OPTTABLE_STR_TABLE_CODE\n\n";
// Dump prefixes.
OS << "/////////\n";
OS << "// Prefixes\n\n";
OS << "#ifdef OPTTABLE_PREFIXES_TABLE_CODE\n";
- OS << "static constexpr unsigned OptionPrefixesTable[] = {\n";
+ OS << "static constexpr llvm::StringTable::Offset OptionPrefixesTable[] = "
+ "{\n";
{
// Ensure the first prefix set is always empty.
assert(!Prefixes.empty() &&
@@ -339,7 +341,8 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) {
OS << "/////////\n";
OS << "// Prefix Union\n\n";
OS << "#ifdef OPTTABLE_PREFIXES_UNION_CODE\n";
- OS << "static constexpr unsigned OptionPrefixesUnion[] = {\n";
+ OS << "static constexpr llvm::StringTable::Offset OptionPrefixesUnion[] = "
+ "{\n";
{
llvm::ListSeparator Sep(", ");
for (auto Prefix : PrefixesUnion)
>From 01ad3c5b204a63be8a31c3370b092badd6b99e4a Mon Sep 17 00:00:00 2001
From: Chandler Carruth <chandlerc at gmail.com>
Date: Fri, 17 Jan 2025 08:31:45 +0000
Subject: [PATCH 2/3] Switch diagnostic group names to use `llvm::StringTable`
Previously, they used a hand-rolled Pascal-string encoding different
from all the other string tables produced from TableGen. This moves them
to use the newly introduced runtime abstraction, and enhances that
abstraction to support iterating over the string table as used in this
case.
>From what I can tell the Pascal-string encoding isn't critical here to
avoid expensive `strlen` calls, so I think this is a simpler and more
consistent model. But if folks would prefer a Pascal-string style
encoding, I can instead work to switch the `StringTable` abstraction
towards that. It would require some tricky tradeoffs though to make it
reasonably general: either using 4 bytes instead of 1 byte to encode the
size, or having a fallback to `strlen` for long strings.
---
clang/lib/Basic/DiagnosticIDs.cpp | 18 +++----
clang/tools/diagtool/DiagnosticNames.cpp | 3 +-
.../TableGen/ClangDiagnosticsEmitter.cpp | 28 ++++-------
llvm/include/llvm/ADT/StringTable.h | 50 ++++++++++++++++++-
4 files changed, 69 insertions(+), 30 deletions(-)
diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp
index d77f28c80b2eb2..55f868147134b7 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -16,6 +16,7 @@
#include "clang/Basic/SourceManager.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringTable.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Path.h"
#include <map>
@@ -618,11 +619,7 @@ namespace {
uint16_t SubGroups;
StringRef Documentation;
- // String is stored with a pascal-style length byte.
- StringRef getName() const {
- return StringRef(DiagGroupNames + NameOffset + 1,
- DiagGroupNames[NameOffset]);
- }
+ StringRef getName() const { return DiagGroupNames[NameOffset]; }
};
}
@@ -669,11 +666,12 @@ StringRef DiagnosticIDs::getWarningOptionForDiag(unsigned DiagID) {
std::vector<std::string> DiagnosticIDs::getDiagnosticFlags() {
std::vector<std::string> Res{"-W", "-Wno-"};
- for (size_t I = 1; DiagGroupNames[I] != '\0';) {
- std::string Diag(DiagGroupNames + I + 1, DiagGroupNames[I]);
- I += DiagGroupNames[I] + 1;
- Res.push_back("-W" + Diag);
- Res.push_back("-Wno-" + Diag);
+ for (StringRef Name : DiagGroupNames) {
+ if (Name.empty())
+ continue;
+
+ Res.push_back((Twine("-W") + Name).str());
+ Res.push_back((Twine("-Wno-") + Name).str());
}
return Res;
diff --git a/clang/tools/diagtool/DiagnosticNames.cpp b/clang/tools/diagtool/DiagnosticNames.cpp
index eb90f082437b33..1004e7bf2063b1 100644
--- a/clang/tools/diagtool/DiagnosticNames.cpp
+++ b/clang/tools/diagtool/DiagnosticNames.cpp
@@ -9,6 +9,7 @@
#include "DiagnosticNames.h"
#include "clang/Basic/AllDiagnostics.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringTable.h"
using namespace clang;
using namespace diagtool;
@@ -74,7 +75,7 @@ static const GroupRecord OptionTable[] = {
};
llvm::StringRef GroupRecord::getName() const {
- return StringRef(DiagGroupNames + NameOffset + 1, DiagGroupNames[NameOffset]);
+ return DiagGroupNames[NameOffset];
}
GroupRecord::subgroup_iterator GroupRecord::subgroup_begin() const {
diff --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
index fb00c640d6b144..5f03efdb804344 100644
--- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -1782,19 +1782,12 @@ static void emitDiagArrays(DiagsInGroupTy &DiagsInGroup,
/// Emit a list of group names.
///
-/// This creates a long string which by itself contains a list of pascal style
-/// strings, which consist of a length byte directly followed by the string.
-///
-/// \code
-/// static const char DiagGroupNames[] = {
-/// \000\020#pragma-messages\t#warnings\020CFString-literal"
-/// };
-/// \endcode
+/// This creates an `llvm::StringTable` of all the diagnostic group names.
static void emitDiagGroupNames(const StringToOffsetTable &GroupNames,
raw_ostream &OS) {
- OS << "static const char DiagGroupNames[] = {\n";
- GroupNames.EmitString(OS);
- OS << "};\n\n";
+ GroupNames.EmitStringLiteralDef(
+ OS, "static constexpr llvm::StringTable DiagGroupNames");
+ OS << "\n";
}
/// Emit diagnostic arrays and related data structures.
@@ -1806,7 +1799,7 @@ static void emitDiagGroupNames(const StringToOffsetTable &GroupNames,
/// #ifdef GET_DIAG_ARRAYS
/// static const int16_t DiagArrays[];
/// static const int16_t DiagSubGroups[];
-/// static const char DiagGroupNames[];
+/// static constexpr llvm::StringTable DiagGroupNames;
/// #endif
/// \endcode
static void emitAllDiagArrays(DiagsInGroupTy &DiagsInGroup,
@@ -1858,9 +1851,7 @@ static void emitDiagTable(DiagsInGroupTy &DiagsInGroup,
"0123456789!@#$%^*-+=:?") != std::string::npos)
PrintFatalError("Invalid character in diagnostic group '" + Name + "'");
OS << Name << " */, ";
- // Store a pascal-style length byte at the beginning of the string.
- std::string PascalName = char(Name.size()) + Name.str();
- OS << *GroupNames.GetStringOffset(PascalName) << ", ";
+ OS << *GroupNames.GetStringOffset(Name) << ", ";
// Special handling for 'pedantic'.
const bool IsPedantic = Name == "pedantic";
@@ -1948,10 +1939,11 @@ void clang::EmitClangDiagGroups(const RecordKeeper &Records, raw_ostream &OS) {
inferPedantic.compute(&DiagsInPedantic, &GroupsInPedantic);
StringToOffsetTable GroupNames;
+ // Add an empty string to the table first so we can use `llvm::StringTable`.
+ // TODO: Factor this into `StringToOffsetTable`.
+ GroupNames.GetOrAddStringOffset("");
for (const auto &[Name, Group] : DiagsInGroup) {
- // Store a pascal-style length byte at the beginning of the string.
- std::string PascalName = char(Name.size()) + Name.str();
- GroupNames.GetOrAddStringOffset(PascalName, false);
+ GroupNames.GetOrAddStringOffset(Name);
}
emitAllDiagArrays(DiagsInGroup, DiagsInPedantic, GroupsInPedantic, GroupNames,
diff --git a/llvm/include/llvm/ADT/StringTable.h b/llvm/include/llvm/ADT/StringTable.h
index 4049f892fa66e0..5f9ecc452808de 100644
--- a/llvm/include/llvm/ADT/StringTable.h
+++ b/llvm/include/llvm/ADT/StringTable.h
@@ -10,6 +10,8 @@
#define LLVM_ADT_STRING_TABLE_H
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator.h"
+#include <iterator>
#include <limits>
namespace llvm {
@@ -51,6 +53,14 @@ class StringTable {
constexpr Offset() = default;
constexpr Offset(unsigned Value) : Value(Value) {}
+ friend constexpr bool operator==(const Offset &LHS, const Offset &RHS) {
+ return LHS.Value == RHS.Value;
+ }
+
+ friend constexpr bool operator!=(const Offset &LHS, const Offset &RHS) {
+ return LHS.Value != RHS.Value;
+ }
+
constexpr unsigned value() const { return Value; }
};
@@ -69,9 +79,13 @@ class StringTable {
assert(!Table.empty() && "Requires at least a valid empty string.");
assert(Table.data()[0] == '\0' && "Offset zero must be the empty string.");
// Ensure that `strlen` from any offset cannot overflow the end of the table
- // by insisting on a null byte at the end.
+ // by insisting on a null byte at the end. We also insist on the last string
+ // within the table being *separately* null terminated. This structure is
+ // used to enable predictable iteration over all the strings when needed.
assert(Table.data()[Table.size() - 1] == '\0' &&
"Last byte must be a null byte.");
+ assert(Table.data()[Table.size() - 2] == '\0' &&
+ "Next-to-last byte must be a null byte.");
}
// Get a string from the table starting with the provided offset. The returned
@@ -84,6 +98,40 @@ class StringTable {
/// Returns the byte size of the table.
constexpr size_t size() const { return Table.size(); }
+
+ class Iterator
+ : public iterator_facade_base<Iterator, std::forward_iterator_tag,
+ const StringRef> {
+ friend StringTable;
+
+ const StringTable *Table;
+ Offset O;
+
+ // A cache of one value to allow `*` to return a reference.
+ mutable StringRef S;
+
+ explicit constexpr Iterator(const StringTable &Table, Offset O)
+ : Table(&Table), O(O) {}
+
+ public:
+ constexpr Iterator(const Iterator &RHS) = default;
+ constexpr Iterator(Iterator &&RHS) = default;
+
+ bool operator==(const Iterator &RHS) const {
+ assert(Table == RHS.Table && "Compared iterators for unrelated tables!");
+ return O == RHS.O;
+ }
+
+ const StringRef &operator*() const { return (S = (*Table)[O]); }
+
+ Iterator &operator++() {
+ O = O.value() + (*Table)[O].size() + 1;
+ return *this;
+ }
+ };
+
+ constexpr Iterator begin() const { return Iterator(*this, 0); }
+ constexpr Iterator end() const { return Iterator(*this, size() - 1); }
};
} // namespace llvm
>From a72cfba4c85bc14ea81edd93de56d66917046193 Mon Sep 17 00:00:00 2001
From: Chandler Carruth <chandlerc at gmail.com>
Date: Thu, 16 Jan 2025 21:29:40 +0000
Subject: [PATCH 3/3] [StrTable] Switch intrinsics to `StringTable` and work
around MSVC
Historically, the main example of *very* large string tables used the
`EmitCharArray` to work around MSVC limitations with string literals,
but that was switched (without removing the API) in order to consolidate
on a nicer emission primitive.
While this large string table in `IntrinsicsImpl.inc` seems to compile
correctly on MSVC without the work around in `EmitCharArray` (and that
this PR adds back to the nicer emission path), other users have
repeatedly hit this MSVC limitation as you can see in the discussion on
PR #120534. This PR teaches the string offset table emission to look at
the size of the table and switch to the char array emission strategy
when the table becomes too large.
This work around does have the downside of making compile times worse
for large string tables, but that appears unavoidable until we can
identify known good MSVC versions and switch to requiring them for all
LLVM users. It also reduces searchability of the generated string table
-- I looked at emitting a comment with each string but it is tricky
because the escaping rules for an inline comment are different from
those of of a string literal, and there's no real way to turn the string
literal into a comment.
This PR also switches the `IntrinsicsImpl.inc` string tables over to the
new `StringTable` runtime abstraction. I didn't want to do this until
landing the MSVC workaround in case it caused even this example to start
hitting the MSVC bug, but I wanted to switch here so that I could
simplify the API for emitting the string table with the workaround
present. With the two different emission strategies, its important to
use a very exact syntax and that seems better encapsulated in the API.
This PR should unblock landing #120534 and letting us switch all of
Clang's builtins to use string tables. That PR has all the details
motivating the overall effort.
Follow-up patches will try to consolidate the remaining users onto the
single interface, but those at least were easy to separate into
follow-ups and keep this PR somewhat smaller.
---
.../TableGen/ClangDiagnosticsEmitter.cpp | 6 +-
.../llvm/TableGen/StringToOffsetTable.h | 82 ++++++++++++-------
llvm/lib/IR/Intrinsics.cpp | 19 +++--
llvm/test/TableGen/MixedCasedMnemonic.td | 2 +-
.../utils/TableGen/Basic/IntrinsicEmitter.cpp | 7 +-
llvm/utils/TableGen/OptionParserEmitter.cpp | 8 +-
6 files changed, 67 insertions(+), 57 deletions(-)
diff --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
index 5f03efdb804344..50dbe4d5a8cab7 100644
--- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -1785,8 +1785,7 @@ static void emitDiagArrays(DiagsInGroupTy &DiagsInGroup,
/// This creates an `llvm::StringTable` of all the diagnostic group names.
static void emitDiagGroupNames(const StringToOffsetTable &GroupNames,
raw_ostream &OS) {
- GroupNames.EmitStringLiteralDef(
- OS, "static constexpr llvm::StringTable DiagGroupNames");
+ GroupNames.EmitStringTableDef(OS, "DiagGroupNames");
OS << "\n";
}
@@ -1939,9 +1938,6 @@ void clang::EmitClangDiagGroups(const RecordKeeper &Records, raw_ostream &OS) {
inferPedantic.compute(&DiagsInPedantic, &GroupsInPedantic);
StringToOffsetTable GroupNames;
- // Add an empty string to the table first so we can use `llvm::StringTable`.
- // TODO: Factor this into `StringToOffsetTable`.
- GroupNames.GetOrAddStringOffset("");
for (const auto &[Name, Group] : DiagsInGroup) {
GroupNames.GetOrAddStringOffset(Name);
}
diff --git a/llvm/include/llvm/TableGen/StringToOffsetTable.h b/llvm/include/llvm/TableGen/StringToOffsetTable.h
index d4bb685acce327..9cec97ed31848d 100644
--- a/llvm/include/llvm/TableGen/StringToOffsetTable.h
+++ b/llvm/include/llvm/TableGen/StringToOffsetTable.h
@@ -27,6 +27,12 @@ class StringToOffsetTable {
std::string AggregateString;
public:
+ StringToOffsetTable() {
+ // Ensure we always put the empty string at offset zero. That lets empty
+ // initialization also be zero initialization for offsets into the table.
+ GetOrAddStringOffset("");
+ }
+
bool empty() const { return StringOffset.empty(); }
size_t size() const { return AggregateString.size(); }
@@ -51,28 +57,62 @@ class StringToOffsetTable {
return II->second;
}
- // Emit the string using string literal concatenation, for better readability
- // and searchability.
- void EmitStringLiteralDef(raw_ostream &OS, const Twine &Decl,
- const Twine &Indent = " ") const {
+ // Emit a string table definition with the provided name and indent.
+ //
+ // When possible, this uses string-literal concatenation to emit the string
+ // contents in a readable and searchable way. However, for (very) large string
+ // tables MSVC cannot reliably use string literals and so there we use a large
+ // character array. We still use a line oriented emission and add comments to
+ // provide searchability even in this case.
+ //
+ // The string table, and its input string contents, are always emitted as both
+ // `static` and `constexpr`. Both `Name` and (`Name` + "Storage") must be
+ // valid identifiers to declare.
+ void EmitStringTableDef(raw_ostream &OS, const Twine &Name,
+ const Twine &Indent = "") const {
OS << formatv(R"(
#ifdef __GNUC__
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Woverlength-strings"
#endif
-{0}{1} = )",
- Indent, Decl);
+{0}static constexpr char {1}Storage[] = )",
+ Indent, Name);
+ // MSVC silently miscompiles string literals longer than 64k in some
+ // circumstances. When the string table is longer, emit it as an array of
+ // character literals.
+ bool UseChars = AggregateString.size() > (64 * 1024);
+ OS << (UseChars ? "{\n" : "\n");
+
+ llvm::ListSeparator LineSep(UseChars ? ",\n" : "\n");
for (StringRef Str : split(AggregateString, '\0')) {
- OS << "\n" << Indent << " \"";
- OS.write_escaped(Str);
- OS << "\\0\"";
+ OS << LineSep << Indent << " ";
+ // If we can, just emit this as a string literal to be concatenated.
+ if (!UseChars) {
+ OS << "\"";
+ OS.write_escaped(Str);
+ OS << "\\0\"";
+ continue;
+ }
+
+ llvm::ListSeparator CharSep(", ");
+ for (char C : Str) {
+ OS << CharSep << "'";
+ OS.write_escaped(StringRef(&C, 1));
+ OS << "'";
+ }
+ OS << CharSep << "'\\0'";
}
- OS << R"(;
+ OS << LineSep << Indent << (UseChars ? "};" : " ;");
+
+ OS << formatv(R"(
#ifdef __GNUC__
#pragma GCC diagnostic pop
#endif
-)";
+
+{0}static constexpr llvm::StringTable {1} = {1}Storage;
+)",
+ Indent, Name);
}
// Emit the string as one single string.
@@ -110,26 +150,6 @@ class StringToOffsetTable {
}
O << "\"";
}
-
- /// Emit the string using character literals. MSVC has a limitation that
- /// string literals cannot be longer than 64K.
- void EmitCharArray(raw_ostream &O) {
- assert(AggregateString.find(')') == std::string::npos &&
- "can't emit raw string with closing parens");
- int Count = 0;
- O << ' ';
- for (char C : AggregateString) {
- O << " \'";
- O.write_escaped(StringRef(&C, 1));
- O << "\',";
- Count++;
- if (Count > 14) {
- O << "\n ";
- Count = 0;
- }
- }
- O << '\n';
- }
};
} // end namespace llvm
diff --git a/llvm/lib/IR/Intrinsics.cpp b/llvm/lib/IR/Intrinsics.cpp
index ec1184e8d835d6..be8f33dc22f546 100644
--- a/llvm/lib/IR/Intrinsics.cpp
+++ b/llvm/lib/IR/Intrinsics.cpp
@@ -12,6 +12,7 @@
#include "llvm/IR/Intrinsics.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringTable.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/IntrinsicsAArch64.h"
#include "llvm/IR/IntrinsicsAMDGPU.h"
@@ -40,7 +41,7 @@ using namespace llvm;
StringRef Intrinsic::getBaseName(ID id) {
assert(id < num_intrinsics && "Invalid intrinsic ID!");
- return IntrinsicNameTable + IntrinsicNameOffsetTable[id];
+ return IntrinsicNameTable[IntrinsicNameOffsetTable[id]];
}
StringRef Intrinsic::getName(ID id) {
@@ -649,20 +650,20 @@ static int lookupLLVMIntrinsicByName(ArrayRef<unsigned> NameOffsetTable,
// `equal_range` requires the comparison to work with either side being an
// offset or the value. Detect which kind each side is to set up the
// compared strings.
- const char *LHSStr;
+ StringRef LHSStr;
if constexpr (std::is_integral_v<decltype(LHS)>) {
- LHSStr = &IntrinsicNameTable[LHS];
+ LHSStr = IntrinsicNameTable[LHS];
} else {
LHSStr = LHS;
}
- const char *RHSStr;
+ StringRef RHSStr;
if constexpr (std::is_integral_v<decltype(RHS)>) {
- RHSStr = &IntrinsicNameTable[RHS];
+ RHSStr = IntrinsicNameTable[RHS];
} else {
RHSStr = RHS;
}
- return strncmp(LHSStr + CmpStart, RHSStr + CmpStart, CmpEnd - CmpStart) <
- 0;
+ return strncmp(LHSStr.data() + CmpStart, RHSStr.data() + CmpStart,
+ CmpEnd - CmpStart) < 0;
};
LastLow = Low;
std::tie(Low, High) = std::equal_range(Low, High, Name.data(), Cmp);
@@ -672,7 +673,7 @@ static int lookupLLVMIntrinsicByName(ArrayRef<unsigned> NameOffsetTable,
if (LastLow == NameOffsetTable.end())
return -1;
- StringRef NameFound = &IntrinsicNameTable[*LastLow];
+ StringRef NameFound = IntrinsicNameTable[*LastLow];
if (Name == NameFound ||
(Name.starts_with(NameFound) && Name[NameFound.size()] == '.'))
return LastLow - NameOffsetTable.begin();
@@ -716,7 +717,7 @@ Intrinsic::ID Intrinsic::lookupIntrinsicID(StringRef Name) {
// If the intrinsic is not overloaded, require an exact match. If it is
// overloaded, require either exact or prefix match.
- const auto MatchSize = strlen(&IntrinsicNameTable[NameOffsetTable[Idx]]);
+ const auto MatchSize = IntrinsicNameTable[NameOffsetTable[Idx]].size();
assert(Name.size() >= MatchSize && "Expected either exact or prefix match");
bool IsExactMatch = Name.size() == MatchSize;
return IsExactMatch || Intrinsic::isOverloaded(ID) ? ID
diff --git a/llvm/test/TableGen/MixedCasedMnemonic.td b/llvm/test/TableGen/MixedCasedMnemonic.td
index cb224ac59c6de5..14ab104c7e1203 100644
--- a/llvm/test/TableGen/MixedCasedMnemonic.td
+++ b/llvm/test/TableGen/MixedCasedMnemonic.td
@@ -41,7 +41,7 @@ def :MnemonicAlias<"InstB", "BInst">;
// Check that the matcher lower()s the mnemonics it matches.
// MATCHER: static const char MnemonicTable[] =
-// MATCHER-NEXT: "\005ainst\005binst";
+// MATCHER-NEXT: "\000\005ainst\005binst";
// Check that aInst appears before BInst in the match table.
// This shows that the mnemonics are sorted in a case-insensitive way,
diff --git a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
index fc2b8908a35b84..6b36fddcb4bcec 100644
--- a/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
+++ b/llvm/utils/TableGen/Basic/IntrinsicEmitter.cpp
@@ -252,8 +252,7 @@ void IntrinsicEmitter::EmitIntrinsicToNameTable(
)";
- Table.EmitStringLiteralDef(OS, "static constexpr char IntrinsicNameTable[]",
- /*Indent=*/"");
+ Table.EmitStringTableDef(OS, "IntrinsicNameTable", /*Indent=*/"");
OS << R"(
static constexpr unsigned IntrinsicNameOffsetTable[] = {
@@ -759,13 +758,13 @@ Intrinsic::getIntrinsicFor{}Builtin(StringRef TargetPrefix,
}
if (!Table.empty()) {
- Table.EmitStringLiteralDef(OS, "static constexpr char BuiltinNames[]");
+ Table.EmitStringTableDef(OS, "BuiltinNames");
OS << R"(
struct BuiltinEntry {
ID IntrinsicID;
unsigned StrTabOffset;
- const char *getName() const { return &BuiltinNames[StrTabOffset]; }
+ const char *getName() const { return BuiltinNames[StrTabOffset].data(); }
bool operator<(StringRef RHS) const {
return strncmp(getName(), RHS.data(), RHS.size()) < 0;
}
diff --git a/llvm/utils/TableGen/OptionParserEmitter.cpp b/llvm/utils/TableGen/OptionParserEmitter.cpp
index 35a452890b0ec7..d17cad41e6a7eb 100644
--- a/llvm/utils/TableGen/OptionParserEmitter.cpp
+++ b/llvm/utils/TableGen/OptionParserEmitter.cpp
@@ -287,10 +287,6 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) {
array_pod_sort(PrefixesUnion.begin(), PrefixesUnion.end());
llvm::StringToOffsetTable Table;
- // Make sure the empty string is the zero-th one in the table. This both makes
- // it easy to check for empty strings (zero offset == empty) and makes
- // initialization cheaper for empty strings.
- Table.GetOrAddStringOffset("");
// We can add all the prefixes via the union.
for (const auto &Prefix : PrefixesUnion)
Table.GetOrAddStringOffset(Prefix);
@@ -303,9 +299,7 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) {
OS << "/////////\n";
OS << "// String table\n\n";
OS << "#ifdef OPTTABLE_STR_TABLE_CODE\n";
- Table.EmitStringLiteralDef(
- OS, "static constexpr llvm::StringTable OptionStrTable",
- /*Indent=*/"");
+ Table.EmitStringTableDef(OS, "OptionStrTable", /*Indent=*/"");
OS << "#endif // OPTTABLE_STR_TABLE_CODE\n\n";
// Dump prefixes.
More information about the lldb-commits
mailing list