[clang] [clang-tools-extra] [llvm] [OptTable] Store Info strings in the string table (PR #218845)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 25 23:59:04 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangd
Author: Fangrui Song (MaskRay)
<details>
<summary>Changes</summary>
Change HelpText, MetaVar, AliasArgs, and Values from `const char *` to
StringTable::Offset, making the fields smaller, and removing dynamic
relocations in .data.rel.ro in PIC links.
Values declared with ValuesCode are computed by generated code that
TableGen cannot see, so they move into a separate OptionValuesCodeTable;
only clang has any.
An empty HelpText is now indistinguishable from an absent one, so an
alias with HelpText<""> no longer suppresses the aliased option's help
text under ShowAllAliases.
Aided by Opus 5
---
Patch is 27.52 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/218845.diff
9 Files Affected:
- (modified) clang-tools-extra/clangd/CompileCommands.cpp (+2-2)
- (modified) clang/lib/Options/DriverOptions.cpp (+3-1)
- (modified) llvm/include/llvm/Option/OptTable.h (+40-18)
- (modified) llvm/include/llvm/Option/Option.h (+12-7)
- (modified) llvm/lib/Option/OptTable.cpp (+25-20)
- (modified) llvm/lib/Option/Option.cpp (+4-10)
- (modified) llvm/unittests/Option/OptionParsingTest.cpp (+11)
- (modified) llvm/unittests/Option/Opts.td (+1)
- (modified) llvm/utils/TableGen/OptionParserEmitter.cpp (+78-54)
``````````diff
diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index b5965d163d7db..2ba9446aa57dc 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -495,7 +495,7 @@ llvm::ArrayRef<ArgStripper::Rule> ArgStripper::rulesFor(llvm::StringRef Arg) {
struct {
DriverID ID;
DriverID AliasID;
- const void *AliasArgs;
+ unsigned AliasArgsOffset;
} AliasTable[] = {
#define OPTION(PREFIX, PREFIXED_NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, \
FLAGS, VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, \
@@ -505,7 +505,7 @@ llvm::ArrayRef<ArgStripper::Rule> ArgStripper::rulesFor(llvm::StringRef Arg) {
#undef OPTION
};
for (auto &E : AliasTable)
- if (E.AliasID != DriverID::OPT_INVALID && E.AliasArgs == nullptr)
+ if (E.AliasID != DriverID::OPT_INVALID && !E.AliasArgsOffset)
AddAlias(E.ID, E.AliasID);
auto Result = std::make_unique<TableTy>();
diff --git a/clang/lib/Options/DriverOptions.cpp b/clang/lib/Options/DriverOptions.cpp
index c9c826a439123..133fc0babae14 100644
--- a/clang/lib/Options/DriverOptions.cpp
+++ b/clang/lib/Options/DriverOptions.cpp
@@ -41,7 +41,9 @@ class DriverOptTable : public PrecomputedOptTable {
public:
DriverOptTable()
: PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable,
- OptionPrefixesUnion) {}
+ OptionPrefixesUnion, /*IgnoreCase=*/false,
+ /*SubCommands=*/{}, /*SubCommandIDsTable=*/{},
+ OptionValuesCodeTable) {}
};
} // anonymous namespace
diff --git a/llvm/include/llvm/Option/OptTable.h b/llvm/include/llvm/Option/OptTable.h
index 45083b31c11f4..58be2bda0d30a 100644
--- a/llvm/include/llvm/Option/OptTable.h
+++ b/llvm/include/llvm/Option/OptTable.h
@@ -60,11 +60,14 @@ class LLVM_ABI OptTable {
const char *Usage;
};
+ /// An option ID together with the values produced by TableGen `ValuesCode`.
+ using ValuesCodeEntry = std::pair<unsigned, const char *>;
+
/// Entry for a single option instance in the option data table.
struct Info {
unsigned PrefixesOffset;
StringTable::Offset PrefixedNameOffset;
- const char *HelpText;
+ StringTable::Offset HelpTextOffset;
// Help text for specific visibilities. A list of pairs, where each pair
// is a list of visibilities and a specific help string for those
// visibilities. If no help text is found in this list for the visibility of
@@ -73,10 +76,10 @@ class LLVM_ABI OptTable {
// here if you need more entries and adjust the constants in
// OptionParserEmitter::EmitHelpTextsForVariants.
std::array<std::pair<std::array<unsigned int, 2 /*MaxVisibilityPerHelp*/>,
- const char *>,
+ StringTable::Offset>,
1 /*MaxVisibilityHelp*/>
HelpTextsForVariants;
- const char *MetaVar;
+ StringTable::Offset MetaVarOffset;
unsigned ID;
unsigned char Kind;
unsigned char Param;
@@ -84,8 +87,11 @@ class LLVM_ABI OptTable {
unsigned int Visibility;
unsigned short GroupID;
unsigned short AliasID;
- const char *AliasArgs;
- const char *Values;
+ /// The alias arguments as a \0 separated list terminated by an empty
+ /// string, e.g. "foo\0bar\0".
+ StringTable::Offset AliasArgsOffset;
+ /// The possible values as a comma separated list; see ValuesCodeTable.
+ StringTable::Offset ValuesOffset;
// Offset into OptTable's SubCommandIDsTable.
unsigned SubCommandIDsOffset;
@@ -179,6 +185,10 @@ class LLVM_ABI OptTable {
/// The subcommand IDs table.
ArrayRef<unsigned> SubCommandIDsTable;
+ /// Values of options declared with TableGen `ValuesCode`: only the generated
+ /// code knows them, so they cannot go in the string table.
+ ArrayRef<ValuesCodeEntry> ValuesCodeTable;
+
bool GroupedShortOptions = false;
bool DashDashParsing = false;
const char *EnvVar = nullptr;
@@ -205,6 +215,15 @@ class LLVM_ABI OptTable {
return OptionInfos[id - 1];
}
+ StringRef getOptionValues(const Info &I) const {
+ if (I.ValuesOffset.value())
+ return (*StrTable)[I.ValuesOffset];
+ for (const auto &[ID, Values] : ValuesCodeTable)
+ if (ID == I.ID)
+ return Values;
+ return StringRef();
+ }
+
std::unique_ptr<Arg> parseOneArgGrouped(InputArgList &Args,
unsigned &Index) const;
@@ -215,7 +234,8 @@ class LLVM_ABI OptTable {
ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos, bool IgnoreCase = false,
ArrayRef<SubCommand> SubCommands = {},
- ArrayRef<unsigned> SubCommandIDsTable = {});
+ ArrayRef<unsigned> SubCommandIDsTable = {},
+ ArrayRef<ValuesCodeEntry> ValuesCodeTable = {});
/// Build (or rebuild) the PrefixChars member.
void buildPrefixChars();
@@ -276,27 +296,27 @@ class LLVM_ABI OptTable {
}
/// Get the help text to use to describe this option.
- const char *getOptionHelpText(OptSpecifier id) const {
+ StringRef getOptionHelpText(OptSpecifier id) const {
return getOptionHelpText(id, Visibility(0));
}
// Get the help text to use to describe this option.
// If it has visibility specific help text and that visibility is in the
// visibility mask, use that text instead of the generic text.
- const char *getOptionHelpText(OptSpecifier id,
- Visibility VisibilityMask) const {
- auto Info = getInfo(id);
- for (auto [Visibilities, Text] : Info.HelpTextsForVariants)
+ StringRef getOptionHelpText(OptSpecifier id,
+ Visibility VisibilityMask) const {
+ const Info &I = getInfo(id);
+ for (auto [Visibilities, TextOffset] : I.HelpTextsForVariants)
for (auto Visibility : Visibilities)
if (VisibilityMask & Visibility)
- return Text;
- return Info.HelpText;
+ return (*StrTable)[TextOffset];
+ return (*StrTable)[I.HelpTextOffset];
}
/// Get the meta-variable name to use when describing
/// this options values in the help text.
- const char *getOptionMetaVar(OptSpecifier id) const {
- return getInfo(id).MetaVar;
+ StringRef getOptionMetaVar(OptSpecifier id) const {
+ return (*StrTable)[getInfo(id).MetaVarOffset];
}
/// Specify the environment variable where initial options should be read.
@@ -481,7 +501,8 @@ class GenericOptTable : public OptTable {
ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos, bool IgnoreCase = false,
ArrayRef<SubCommand> SubCommands = {},
- ArrayRef<unsigned> SubCommandIDsTable = {});
+ ArrayRef<unsigned> SubCommandIDsTable = {},
+ ArrayRef<ValuesCodeEntry> ValuesCodeTable = {});
};
class PrecomputedOptTable : public OptTable {
@@ -492,9 +513,10 @@ class PrecomputedOptTable : public OptTable {
ArrayRef<StringTable::Offset> PrefixesUnionOffsets,
bool IgnoreCase = false,
ArrayRef<SubCommand> SubCommands = {},
- ArrayRef<unsigned> SubCommandIDsTable = {})
+ ArrayRef<unsigned> SubCommandIDsTable = {},
+ ArrayRef<ValuesCodeEntry> ValuesCodeTable = {})
: OptTable(StrTable, PrefixesTable, OptionInfos, IgnoreCase, SubCommands,
- SubCommandIDsTable) {
+ SubCommandIDsTable, ValuesCodeTable) {
for (auto PrefixOffset : PrefixesUnionOffsets)
PrefixesUnion.push_back(StrTable[PrefixOffset]);
buildPrefixChars();
diff --git a/llvm/include/llvm/Option/Option.h b/llvm/include/llvm/Option/Option.h
index eac964159dc42..53c13239e0943 100644
--- a/llvm/include/llvm/Option/Option.h
+++ b/llvm/include/llvm/Option/Option.h
@@ -117,14 +117,17 @@ class Option {
return Owner->getOption(Info->AliasID);
}
- /// Get the alias arguments as a \0 separated list.
- /// E.g. ["foo", "bar"] would be returned as "foo\0bar\0".
+ /// Get the alias arguments as a \0 separated list terminated by an empty
+ /// string. E.g. ["foo", "bar"] would be returned as "foo\0bar\0".
const char *getAliasArgs() const {
assert(Info && "Must have a valid info!");
- assert((!Info->AliasArgs || Info->AliasArgs[0] != 0) &&
- "AliasArgs should be either 0 or non-empty.");
+ assert(Owner && "Must have a valid owner!");
+ return Owner->getStrTable().getCString(Info->AliasArgsOffset);
+ }
- return Info->AliasArgs;
+ bool hasAliasArgs() const {
+ assert(Info && "Must have a valid info!");
+ return Info->AliasArgsOffset.value() != 0;
}
/// Get the default prefix for this option.
@@ -144,13 +147,15 @@ class Option {
/// Get the help text for this option.
StringRef getHelpText() const {
assert(Info && "Must have a valid info!");
- return Info->HelpText;
+ assert(Owner && "Must have a valid owner!");
+ return Owner->getStrTable()[Info->HelpTextOffset];
}
/// Get the meta-variable list for this option.
StringRef getMetaVar() const {
assert(Info && "Must have a valid info!");
- return Info->MetaVar;
+ assert(Owner && "Must have a valid owner!");
+ return Owner->getStrTable()[Info->MetaVarOffset];
}
unsigned getNumArgs() const { return Info->Param; }
diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index 8444675b847e6..ff1d27d297c4b 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -80,10 +80,12 @@ OptTable::OptTable(const StringTable &StrTable,
ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos, bool IgnoreCase,
ArrayRef<SubCommand> SubCommands,
- ArrayRef<unsigned> SubCommandIDsTable)
+ ArrayRef<unsigned> SubCommandIDsTable,
+ ArrayRef<ValuesCodeEntry> ValuesCodeTable)
: StrTable(&StrTable), PrefixesTable(PrefixesTable),
OptionInfos(OptionInfos), IgnoreCase(IgnoreCase),
- SubCommands(SubCommands), SubCommandIDsTable(SubCommandIDsTable) {
+ SubCommands(SubCommands), SubCommandIDsTable(SubCommandIDsTable),
+ ValuesCodeTable(ValuesCodeTable) {
// Explicitly zero initialize the error to work around a bug in array
// value-initialization on MinGW with gcc 4.3.5.
@@ -193,11 +195,12 @@ 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))
+ StringRef Values = getOptionValues(In);
+ if (Values.empty() || !optionMatches(*StrTable, PrefixesTable, In, Option))
continue;
SmallVector<StringRef, 8> Candidates;
- StringRef(In.Values).split(Candidates, ",", -1, false);
+ Values.split(Candidates, ",", -1, false);
std::vector<std::string> Result;
for (StringRef Val : Candidates)
@@ -214,19 +217,20 @@ OptTable::findByPrefix(StringRef Cur, Visibility VisibilityMask,
std::vector<std::string> Ret;
for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) {
const Info &In = OptionInfos[I];
- if (In.hasNoPrefix() || (!In.HelpText && !In.GroupID))
+ if (In.hasNoPrefix() || (!In.HelpTextOffset.value() && !In.GroupID))
continue;
if (!(In.Visibility & VisibilityMask))
continue;
if (In.Flags & DisableFlags)
continue;
+ StringRef HelpText = (*StrTable)[In.HelpTextOffset];
+
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;
+ S += HelpText;
if (StringRef(S).starts_with(Cur) && S != std::string(Cur) + "\t")
Ret.push_back(S);
}
@@ -616,12 +620,12 @@ static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) {
llvm_unreachable("Invalid option with help text.");
case Option::MultiArgClass:
- if (const char *MetaVarName = Opts.getOptionMetaVar(Id)) {
+ if (StringRef MetaVarName = Opts.getOptionMetaVar(Id);
+ !MetaVarName.empty()) {
// For MultiArgs, metavar is full list of all argument names.
Name += ' ';
Name += MetaVarName;
- }
- else {
+ } else {
// For MultiArgs<N>, if metavar not supplied, print <value> N times.
for (unsigned i=0, e=O.getNumArgs(); i< e; ++i) {
Name += " <value>";
@@ -641,7 +645,7 @@ static std::string getOptionHelpName(const OptTable &Opts, OptSpecifier Id) {
[[fallthrough]];
case Option::JoinedClass: case Option::CommaJoinedClass:
case Option::JoinedAndSeparateClass:
- if (const char *MetaVarName = Opts.getOptionMetaVar(Id))
+ if (StringRef MetaVarName = Opts.getOptionMetaVar(Id); !MetaVarName.empty())
Name += MetaVarName;
else
Name += "<value>";
@@ -695,7 +699,7 @@ static void PrintHelpOptionList(raw_ostream &OS, StringRef Title,
}
}
-static const char *getOptionHelpGroup(const OptTable &Opts, OptSpecifier Id) {
+static StringRef getOptionHelpGroup(const OptTable &Opts, OptSpecifier Id) {
unsigned GroupID = Opts.getOptionGroupID(Id);
// If not in a group, return the default help group.
@@ -706,7 +710,7 @@ static const char *getOptionHelpGroup(const OptTable &Opts, OptSpecifier Id) {
// name.
//
// FIXME: Split out option groups.
- if (const char *GroupHelp = Opts.getOptionHelpText(GroupID))
+ if (StringRef GroupHelp = Opts.getOptionHelpText(GroupID); !GroupHelp.empty())
return GroupHelp;
// Otherwise keep looking.
@@ -814,17 +818,17 @@ void OptTable::internalPrintHelp(
// If an alias doesn't have a help text, show a help text for the aliased
// option instead.
- const char *HelpText = getOptionHelpText(Id, VisibilityMask);
- if (!HelpText && ShowAllAliases) {
+ StringRef HelpText = getOptionHelpText(Id, VisibilityMask);
+ if (HelpText.empty() && ShowAllAliases) {
const Option Alias = getOption(Id).getAlias();
if (Alias.isValid())
HelpText = getOptionHelpText(Alias.getID(), VisibilityMask);
}
- if (HelpText && (strlen(HelpText) != 0)) {
- const char *HelpGroup = getOptionHelpGroup(*this, Id);
+ if (!HelpText.empty()) {
+ StringRef HelpGroup = getOptionHelpGroup(*this, Id);
const std::string &OptName = getOptionHelpName(*this, Id);
- GroupedOptionHelp[HelpGroup].push_back({OptName, HelpText});
+ GroupedOptionHelp[std::string(HelpGroup)].push_back({OptName, HelpText});
}
}
@@ -841,9 +845,10 @@ GenericOptTable::GenericOptTable(const StringTable &StrTable,
ArrayRef<StringTable::Offset> PrefixesTable,
ArrayRef<Info> OptionInfos, bool IgnoreCase,
ArrayRef<SubCommand> SubCommands,
- ArrayRef<unsigned> SubCommandIDsTable)
+ ArrayRef<unsigned> SubCommandIDsTable,
+ ArrayRef<ValuesCodeEntry> ValuesCodeTable)
: OptTable(StrTable, PrefixesTable, OptionInfos, IgnoreCase, SubCommands,
- SubCommandIDsTable) {
+ SubCommandIDsTable, ValuesCodeTable) {
std::set<StringRef> TmpPrefixesUnion;
for (auto const &Info : OptionInfos.drop_front(FirstSearchableIndex))
diff --git a/llvm/lib/Option/Option.cpp b/llvm/lib/Option/Option.cpp
index 838dd344b18a9..306c96c9d564a 100644
--- a/llvm/lib/Option/Option.cpp
+++ b/llvm/lib/Option/Option.cpp
@@ -29,7 +29,7 @@ Option::Option(const OptTable::Info *Info, const OptTable *Owner)
assert((!Info || !getAlias().isValid() || !getAlias().getAlias().isValid()) &&
"Multi-level aliases are not supported.");
- if (Info && getAliasArgs()) {
+ if (Info && hasAliasArgs()) {
assert(getAlias().isValid() && "Only alias options can have alias args.");
assert(getKind() == FlagClass && "Only Flag aliases can have alias args.");
assert(getAlias().getKind() != FlagClass &&
@@ -281,15 +281,9 @@ std::unique_ptr<Arg> Option::accept(const ArgList &Args, StringRef CurArg,
}
// FlagClass aliases can have AliasArgs<>; add those to the unaliased arg.
- if (const char *Val = getAliasArgs()) {
- while (*Val != '\0') {
- UnaliasedA->getValues().push_back(Val);
-
- // Move past the '\0' to the next argument.
- Val += strlen(Val) + 1;
- }
- }
- if (UnaliasedOption.getKind() == JoinedClass && !getAliasArgs())
+ for (const char *Val = getAliasArgs(); *Val; Val += strlen(Val) + 1)
+ UnaliasedA->getValues().push_back(Val);
+ if (UnaliasedOption.getKind() == JoinedClass && !hasAliasArgs())
// A Flag alias for a Joined option must provide an argument.
UnaliasedA->getValues().push_back("");
return UnaliasedA;
diff --git a/llvm/unittests/Option/OptionParsingTest.cpp b/llvm/unittests/Option/OptionParsingTest.cpp
index 3da015e343eb9..51030ee7018b9 100644
--- a/llvm/unittests/Option/OptionParsingTest.cpp
+++ b/llvm/unittests/Option/OptionParsingTest.cpp
@@ -229,6 +229,17 @@ TYPED_TEST(OptTableTest, AliasArgs) {
EXPECT_EQ("bar", AL.getAllArgValues(OPT_B)[1]);
}
+TYPED_TEST(OptTableTest, AliasArgsMultiple) {
+ TypeParam T;
+ unsigned MAI, MAC;
+
+ const char *MyArgs[] = {"-Jmulti"};
+ InputArgList AL = T.ParseArgs(MyArgs, MAI, MAC);
+ EXPECT_TRUE(AL.hasArg(OPT_D));
+ EXPECT_EQ((std::vector<std::string>{"foo", "bar"}),
+ AL.getAllArgValues(OPT_D));
+}
+
TYPED_TEST(OptTableTest, IgnoreCase) {
TypeParam T(true);
unsigned MAI, MAC;
diff --git a/llvm/unittests/Option/Opts.td b/llvm/unittests/Option/Opts.td
index 5be67c9decdbc..2c6c552232ebe 100644
--- a/llvm/unittests/Option/Opts.td
+++ b/llvm/unittests/Option/Opts.td
@@ -26,6 +26,7 @@ def I : Flag<["-"], "I">, Alias<H>, Group<my_group>;
def J : Flag<["-"], "J">, Alias<B>, AliasArgs<["foo"]>;
def Joo : Flag<["-"], "Joo">, Alias<B>, AliasArgs<["bar"]>;
+def Jmulti : Flag<["-"], "Jmulti">, Alias<D>, AliasArgs<["foo", "bar"]>;
def K : Flag<["-"], "K">, Alias<B>;
diff --git a/llvm/utils/TableGen/OptionParserEmitter.cpp b/llvm/utils/TableGen/OptionParserEmitter.cpp
index 829c202b495e4..98741d3c0d402 100644
--- a/llvm/utils/TableGen/OptionParserEmitter.cpp
+++ b/llvm/utils/TableGen/OptionParserEmitter.cpp
@@ -31,12 +31,17 @@ static std::string getOptionName(const Record &R) {
return R.getValueAsString("EnumName").str();
}
+// Long strings and strings that may contain "*/" need EmitComment=false.
static raw_ostream &writeStrTableOffset(raw_ostream &OS,
const StringToOffsetTable &Table,
- llvm::StringRef Str) {
- OS << Table.GetStringOffset(Str) << " /* ";
- OS.write_escaped(Str);
- OS << " */";
+ llvm::StringRef Str,
+ bool EmitComment = true) {
+ OS << Table.GetStringOffset(Str);
+ if (EmitComment) {
+ OS << " /* ";
+ OS.write_escaped(Str);
+ OS << " */";
+ }
return OS;
}
@@ -47,6 +52,34 @@ static raw_ostream &writeCstring(raw_ostream &OS, llvm::StringRef Str) {
return OS;
}
+// An unset field maps to the empty string, which the table keeps at offset 0.
+static StringRef getOptionalString(const Record &R, StringRef Field) {
+ return R.getValueAsOptionalString(Field).value_or("");
+}
+
+static void addOptionalString(StringToOffsetTable &Table, const Record &R,
+ StringRef Field) {
+ Table.GetOrAddStringOffset(getOptionalString(R, Field));
+}
+
+static void writeOptionalStrOffset(raw_ostream &OS,
+ const StringToOffsetTable &Table,
+ const Record &R, StringRef Field) {
+ writeStrTableOffset(OS, Table, getOptionalString(R, Fiel...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/218845
More information about the llvm-commits
mailing list