llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lld-wasm Author: Fangrui Song (MaskRay) <details> <summary>Changes</summary> Drop ID, which is the entry's 1-based position, and narrow PrefixesOffset and SubCommandIDsOffset to 16 bits like GroupID and AliasID. Move help text variants to a side table. The .td HelpTextForVariants class allows one variant per option and only 5 clang options use it, yet every entry carries a 12-byte std::array of visibility lists. Store (visibility mask, text offset) pairs in a separate table, indexed by a 16-bit field in the entry's padding. clang's table shrinks from 232 KB to 155 KB and Options.inc from 1.70 MB to 1.29 MB. --help output is unchanged. Aided by Opus 5 --- Patch is 23.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/224374.diff 10 Files Affected: - (modified) clang/lib/Options/DriverOptions.cpp (+5) - (modified) lld/MachO/DriverUtils.cpp (+1-18) - (modified) lld/MinGW/Driver.cpp (+1-18) - (modified) lld/wasm/Driver.cpp (+1-18) - (modified) llvm/include/llvm/Option/OptTable.h (+41-30) - (modified) llvm/include/llvm/Option/Option.h (+7-6) - (modified) llvm/lib/Option/OptTable.cpp (+2-2) - (modified) llvm/unittests/Option/OptionParsingTest.cpp (+21) - (modified) llvm/unittests/Option/Opts.td (+2) - (modified) llvm/utils/TableGen/OptionParserEmitter.cpp (+39-52) ``````````diff diff --git a/clang/lib/Options/DriverOptions.cpp b/clang/lib/Options/DriverOptions.cpp index 6f630843f7019a..832c46c4f3eefa 100644 --- a/clang/lib/Options/DriverOptions.cpp +++ b/clang/lib/Options/DriverOptions.cpp @@ -29,6 +29,10 @@ using namespace llvm::opt; #include "clang/Options/Options.inc" #undef OPTTABLE_PREFIXES_UNION_CODE +#define OPTTABLE_HELP_TEXT_VARIANTS_TABLE_CODE +#include "clang/Options/Options.inc" +#undef OPTTABLE_HELP_TEXT_VARIANTS_TABLE_CODE + static constexpr OptTable::Info InfoTable[] = { #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__), #include "clang/Options/Options.inc" @@ -43,6 +47,7 @@ class DriverOptTable : public PrecomputedOptTable { : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, OptionPrefixesUnion) { setValuesCodeFn(getOptionValuesCode); + setHelpTextVariantsTable(OptionHelpTextVariantsTable); } }; } // anonymous namespace diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp index bbfc4477bb978b..46ad681ffaa5d8 100644 --- a/lld/MachO/DriverUtils.cpp +++ b/lld/MachO/DriverUtils.cpp @@ -43,24 +43,7 @@ using namespace lld::macho; // Create table mapping all options defined in Options.td static constexpr OptTable::Info optInfo[] = { -#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \ - VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR, \ - VALUES, SUBCOMMANDIDS_OFFSET) \ - {PREFIX, \ - NAME, \ - HELPTEXT, \ - HELPTEXTSFORVARIANTS, \ - METAVAR, \ - OPT_##ID, \ - opt::Option::KIND##Class, \ - PARAM, \ - FLAGS, \ - VISIBILITY, \ - OPT_##GROUP, \ - OPT_##ALIAS, \ - ALIASARGS, \ - VALUES, \ - SUBCOMMANDIDS_OFFSET}, +#define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__), #include "Options.inc" #undef OPTION }; diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp index c18a74a3068214..11ccb110f76846 100644 --- a/lld/MinGW/Driver.cpp +++ b/lld/MinGW/Driver.cpp @@ -68,24 +68,7 @@ enum { // Create table mapping all options defined in Options.td static constexpr opt::OptTable::Info infoTable[] = { -#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \ - VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR, \ - VALUES, SUBCOMMANDIDS_OFFSET) \ - {PREFIX, \ - NAME, \ - HELPTEXT, \ - HELPTEXTSFORVARIANTS, \ - METAVAR, \ - OPT_##ID, \ - opt::Option::KIND##Class, \ - PARAM, \ - FLAGS, \ - VISIBILITY, \ - OPT_##GROUP, \ - OPT_##ALIAS, \ - ALIASARGS, \ - VALUES, \ - SUBCOMMANDIDS_OFFSET}, +#define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__), #include "Options.inc" #undef OPTION }; diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp index e137336a634d76..e6b3bcc2d438a8 100644 --- a/lld/wasm/Driver.cpp +++ b/lld/wasm/Driver.cpp @@ -155,24 +155,7 @@ bool link(ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS, // Create table mapping all options defined in Options.td static constexpr opt::OptTable::Info optInfo[] = { -#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \ - VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR, \ - VALUES, SUBCOMMANDIDS_OFFSET) \ - {PREFIX, \ - NAME, \ - HELPTEXT, \ - HELPTEXTSFORVARIANTS, \ - METAVAR, \ - OPT_##ID, \ - opt::Option::KIND##Class, \ - PARAM, \ - FLAGS, \ - VISIBILITY, \ - OPT_##GROUP, \ - OPT_##ALIAS, \ - ALIASARGS, \ - VALUES, \ - SUBCOMMANDIDS_OFFSET}, +#define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__), #include "Options.inc" #undef OPTION }; diff --git a/llvm/include/llvm/Option/OptTable.h b/llvm/include/llvm/Option/OptTable.h index ceb22c68ccb3b0..aec49f1fa28a62 100644 --- a/llvm/include/llvm/Option/OptTable.h +++ b/llvm/include/llvm/Option/OptTable.h @@ -65,38 +65,37 @@ class LLVM_ABI OptTable { /// table supplies getOptionValuesCode() for this. using ValuesCodeFnTy = StringRef (*)(unsigned); - /// Entry for a single option instance in the option data table. + /// Help text that replaces Info::HelpTextOffset when the visibility mask + /// being printed intersects Visibility. + struct HelpTextVariant { + unsigned Visibility; + StringTable::Offset HelpTextOffset; + }; + + /// Entry for a single option instance in the option data table. An option's + /// ID is its 1-based position in the table. struct Info { - unsigned PrefixesOffset; StringTable::Offset PrefixedNameOffset; /// Offset 0 means the .td supplied no HelpText. A HelpText<""> maps to a /// distinct empty string, marking the option deliberately undocumented. 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 - // the program, HelpTextOffset is used instead. This cannot use std::vector - // because OptTable is used in constexpr contexts. Increase the array sizes - // here if you need more entries and adjust the constants in - // OptionParserEmitter::EmitHelpTextsForVariants. - std::array<std::pair<std::array<unsigned int, 2 /*MaxVisibilityPerHelp*/>, - StringTable::Offset>, - 1 /*MaxVisibilityHelp*/> - HelpTextsForVariants; StringTable::Offset MetaVarOffset; - unsigned ID; - unsigned char Kind; - unsigned char Param; - unsigned int Flags; - unsigned int Visibility; - unsigned short GroupID; - unsigned short AliasID; StringTable::Offset AliasArgsOffset; /// The possible values as a comma separated list, empty for an option whose /// values only getOptionValuesCode() knows. StringTable::Offset ValuesOffset; + unsigned Flags; + unsigned Visibility; + // Offset into OptTable's PrefixesTable. + unsigned short PrefixesOffset; + unsigned short GroupID; + unsigned short AliasID; + // Index into OptTable's HelpTextVariantsTable; 0 for none. + unsigned short HelpTextVariantsOffset; // Offset into OptTable's SubCommandIDsTable. - unsigned SubCommandIDsOffset; + unsigned short SubCommandIDsOffset; + unsigned char Kind; + unsigned char Param; bool hasNoPrefix() const { return PrefixesOffset == 0; } @@ -191,6 +190,8 @@ class LLVM_ABI OptTable { /// The subcommand IDs table. ArrayRef<unsigned> SubCommandIDsTable; + ArrayRef<HelpTextVariant> HelpTextVariantsTable; + ValuesCodeFnTy ValuesCodeFn = nullptr; bool GroupedShortOptions = false; @@ -221,17 +222,19 @@ class LLVM_ABI OptTable { StringTable::Offset getHelpTextOffset(const Info &I, Visibility VisibilityMask) const { - for (const auto &[Visibilities, TextOffset] : I.HelpTextsForVariants) - for (auto Vis : Visibilities) - if (VisibilityMask & Vis) - return TextOffset; + if (I.HelpTextVariantsOffset) { + const HelpTextVariant &V = + HelpTextVariantsTable[I.HelpTextVariantsOffset]; + if (VisibilityMask & V.Visibility) + return V.HelpTextOffset; + } return I.HelpTextOffset; } StringRef getOptionValues(const Info &I) const { StringRef Values = (*StrTable)[I.ValuesOffset]; if (Values.empty() && ValuesCodeFn) - Values = ValuesCodeFn(I.ID); + Values = ValuesCodeFn(getOptionID(I)); return Values; } @@ -249,6 +252,10 @@ class LLVM_ABI OptTable { void setValuesCodeFn(ValuesCodeFnTy Fn) { ValuesCodeFn = Fn; } + void setHelpTextVariantsTable(ArrayRef<HelpTextVariant> Table) { + HelpTextVariantsTable = Table; + } + /// Build (or rebuild) the PrefixChars member. void buildPrefixChars(); @@ -268,6 +275,10 @@ class LLVM_ABI OptTable { /// Return the total number of option classes. unsigned getNumOptions() const { return OptionInfos.size(); } + unsigned getOptionID(const Info &I) const { + return &I - OptionInfos.data() + 1; + } + /// Get the given Opt's Option instance, lazily creating it /// if necessary. /// @@ -552,10 +563,10 @@ class PrecomputedOptTable : public OptTable { ALIASARGS, FLAGS, VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, \ METAVAR, VALUES, SUBCOMMANDIDS_OFFSET) \ llvm::opt::OptTable::Info { \ - PREFIXES_OFFSET, PREFIXED_NAME_OFFSET, HELPTEXT, HELPTEXTSFORVARIANTS, \ - METAVAR, ID_PREFIX##ID, llvm::opt::Option::KIND##Class, PARAM, FLAGS, \ - VISIBILITY, ID_PREFIX##GROUP, ID_PREFIX##ALIAS, ALIASARGS, VALUES, \ - SUBCOMMANDIDS_OFFSET \ + PREFIXED_NAME_OFFSET, HELPTEXT, METAVAR, ALIASARGS, VALUES, FLAGS, \ + VISIBILITY, PREFIXES_OFFSET, ID_PREFIX##GROUP, ID_PREFIX##ALIAS, \ + HELPTEXTSFORVARIANTS, SUBCOMMANDIDS_OFFSET, \ + llvm::opt::Option::KIND##Class, PARAM \ } #define LLVM_CONSTRUCT_OPT_INFO( \ diff --git a/llvm/include/llvm/Option/Option.h b/llvm/include/llvm/Option/Option.h index 68945c046b642a..dea43ca9c9df68 100644 --- a/llvm/include/llvm/Option/Option.h +++ b/llvm/include/llvm/Option/Option.h @@ -90,7 +90,8 @@ class Option { unsigned getID() const { assert(Info && "Must have a valid info!"); - return Info->ID; + assert(Owner && "Must have a valid owner!"); + return Owner->getOptionID(*Info); } OptionClass getKind() const { @@ -102,7 +103,7 @@ class Option { StringRef getName() const { assert(Info && "Must have a valid info!"); assert(Owner && "Must have a valid owner!"); - return Owner->getOptionName(Info->ID); + return Owner->getOptionName(getID()); } const Option getGroup() const { @@ -134,28 +135,28 @@ class Option { StringRef getPrefix() const { assert(Info && "Must have a valid info!"); assert(Owner && "Must have a valid owner!"); - return Owner->getOptionPrefix(Info->ID); + return Owner->getOptionPrefix(getID()); } /// Get the name of this option with the default prefix. StringRef getPrefixedName() const { assert(Info && "Must have a valid info!"); assert(Owner && "Must have a valid owner!"); - return Owner->getOptionPrefixedName(Info->ID); + return Owner->getOptionPrefixedName(getID()); } /// Get the help text for this option. StringRef getHelpText() const { assert(Info && "Must have a valid info!"); assert(Owner && "Must have a valid owner!"); - return Owner->getOptionHelpText(Info->ID); + return Owner->getOptionHelpText(getID()); } /// Get the meta-variable list for this option. StringRef getMetaVar() const { assert(Info && "Must have a valid info!"); assert(Owner && "Must have a valid owner!"); - return Owner->getOptionMetaVar(Info->ID); + return Owner->getOptionMetaVar(getID()); } unsigned getNumArgs() const { return Info->Param; } diff --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp index ef3a4c694b6c24..15a01f8cf62c6c 100644 --- a/llvm/lib/Option/OptTable.cpp +++ b/llvm/lib/Option/OptTable.cpp @@ -90,10 +90,10 @@ OptTable::OptTable(const StringTable &StrTable, unsigned Kind = getInfo(i + 1).Kind; if (Kind == Option::InputClass) { assert(!InputOptionID && "Cannot have multiple input options!"); - InputOptionID = getInfo(i + 1).ID; + InputOptionID = i + 1; } else if (Kind == Option::UnknownClass) { assert(!UnknownOptionID && "Cannot have multiple unknown options!"); - UnknownOptionID = getInfo(i + 1).ID; + UnknownOptionID = i + 1; } else if (Kind != Option::GroupClass) { FirstSearchableIndex = i; break; diff --git a/llvm/unittests/Option/OptionParsingTest.cpp b/llvm/unittests/Option/OptionParsingTest.cpp index f494ada47b57f0..b6e96cfdeddfa0 100644 --- a/llvm/unittests/Option/OptionParsingTest.cpp +++ b/llvm/unittests/Option/OptionParsingTest.cpp @@ -55,6 +55,10 @@ enum OptionVisibility { MultiLineVis = (1 << 3), }; +#define OPTTABLE_HELP_TEXT_VARIANTS_TABLE_CODE +#include "Opts.inc" +#undef OPTTABLE_HELP_TEXT_VARIANTS_TABLE_CODE + static constexpr OptTable::Info InfoTable[] = { #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__), #include "Opts.inc" @@ -68,6 +72,7 @@ class TestOptTable : public GenericOptTable { : GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, IgnoreCase) { setValuesCodeFn(getOptionValuesCode); + setHelpTextVariantsTable(OptionHelpTextVariantsTable); } }; @@ -77,6 +82,7 @@ class TestPrecomputedOptTable : public PrecomputedOptTable { : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, OptionPrefixesUnion, IgnoreCase) { setValuesCodeFn(getOptionValuesCode); + setHelpTextVariantsTable(OptionHelpTextVariantsTable); } }; } @@ -569,6 +575,21 @@ TYPED_TEST(OptTableTest, UnknownGroupedShortOptions) { EXPECT_EQ("-z", Unknown[3]); } +TYPED_TEST(OptTableTest, HelpTextForVariants) { + TypeParam T; + EXPECT_EQ("The xyzzy2 option", T.getOptionHelpText(OPT_Xyzzy2)); + EXPECT_EQ("The xyzzy2 option", + T.getOptionHelpText(OPT_Xyzzy2, Visibility(SubtoolVis))); + EXPECT_EQ("The xyzzy3 option", T.getOptionHelpText(OPT_Xyzzy3)); + EXPECT_EQ("The xyzzy3 option", + T.getOptionHelpText(OPT_Xyzzy3, Visibility(DefaultVis))); + EXPECT_EQ("The xyzzy3 option for the subtool", + T.getOptionHelpText(OPT_Xyzzy3, Visibility(SubtoolVis))); + EXPECT_EQ( + "The xyzzy3 option for the subtool", + T.getOptionHelpText(OPT_Xyzzy3, Visibility(DefaultVis | SubtoolVis))); +} + TYPED_TEST(OptTableTest, PrintMultilineHelpText) { TypeParam T; std::string Help; diff --git a/llvm/unittests/Option/Opts.td b/llvm/unittests/Option/Opts.td index 89c20359bd2a0b..28e574ceac8d00 100644 --- a/llvm/unittests/Option/Opts.td +++ b/llvm/unittests/Option/Opts.td @@ -46,6 +46,8 @@ def Doopf1 : Flag<["-"], "doopf1">, HelpText<"The doopf1 option">, Flags<[OptFla def Doopf2 : Flag<["-"], "doopf2">, HelpText<"The doopf2 option">, Flags<[OptFlag2]>; def Xyzzy1 : Flag<["-"], "xyzzy1">, HelpText<"The xyzzy1 option">, Visibility<[SubtoolVis]>; def Xyzzy2 : Flag<["-"], "xyzzy2">, HelpText<"The xyzzy2 option">, Visibility<[DefaultVis]>; +def Xyzzy3 : Flag<["-"], "xyzzy3">, HelpText<"The xyzzy3 option">, + HelpTextForVariants<[SubtoolVis], "The xyzzy3 option for the subtool">; def Ermgh : Joined<["--"], "ermgh">, HelpText<"The ermgh option">, MetaVarName<"ERMGH">, Flags<[OptFlag1]>; def Fjormp : Flag<["--"], "fjormp">, HelpText<"The fjormp option">, Flags<[OptFlag1]>; diff --git a/llvm/utils/TableGen/OptionParserEmitter.cpp b/llvm/utils/TableGen/OptionParserEmitter.cpp index ebc5a1a8aa6ccc..a76930e899ccc4 100644 --- a/llvm/utils/TableGen/OptionParserEmitter.cpp +++ b/llvm/utils/TableGen/OptionParserEmitter.cpp @@ -13,7 +13,6 @@ #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/Twine.h" #include "llvm/Option/OptTable.h" -#include "llvm/Support/InterleavedRange.h" #include "llvm/Support/raw_ostream.h" #include "llvm/TableGen/Error.h" #include "llvm/TableGen/Record.h" @@ -228,37 +227,6 @@ static MarshallingInfo createMarshallingInfo(const Record &R) { return Ret; } -static void emitHelpTextsForVariants( - raw_ostream &OS, const StringToOffsetTable &Table, - ArrayRef<std::pair<std::vector<std::string>, StringRef>> - HelpTextsForVariants) { - // OptTable must be constexpr so it uses std::arrays with these capacities. - const unsigned MaxVisibilityPerHelp = 2; - const unsigned MaxVisibilityHelp = 1; - - assert(HelpTextsForVariants.size() <= MaxVisibilityHelp && - "Too many help text variants to store in " - "OptTable::HelpTextsForVariants"); - - OS << ", (std::array<std::pair<std::array<unsigned, " << MaxVisibilityPerHelp - << ">, llvm::StringTable::Offset>, " << MaxVisibilityHelp << ">{{ "; - - ListSeparator Sep; - for (const auto &[Visibilities, Help] : HelpTextsForVariants) { - assert(Visibilities.size() <= MaxVisibilityPerHelp && - "Too many visibilities to store in an " - "OptTable::HelpTextsForVariants entry"); - OS << Sep << "{std::array<unsigned, " << MaxVisibilityPerHelp << ">{{" - << llvm::interleaved(Visibilities) << "}}, "; - writeStrTableOffset(OS, Table, Help); - OS << "}"; - } - // Unused entries are value-initialized. - for (size_t I = HelpTextsForVariants.size(); I < MaxVisibilityHelp; ++I) - OS << Sep << "{}"; - OS << " }})"; -} - /// OptionParserEmitter - This tablegen backend takes an input .td file /// describing a list of options and emits a data structure for parsing and /// working with those options when given an input command line. @@ -329,15 +297,25 @@ static void emitOptionParser(const RecordKeeper &Records, raw_ostream &OS) { Table.GetOrAddStringOffset(R.getValueAsString("Name")); Table.GetOrAddStringOffset(getHelpText(R)); } + // Info::HelpTextVariantsOffset is a 1-based index into this list. + std::vector<const Record *> HelpTextVariants; + DenseMap<const Record *, unsigned> HelpTextVariantIdx; for (const Record &R : llvm::make_pointee_range(Opts)) { Table.GetOrAddStringOffset(getOptionPrefixedName(R)); Table.GetOrAddStringOffset... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/224374 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
