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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
