Skip to content

Rework the Option library to reduce dynamic relocations #119198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

chandlerc
Copy link
Member

@chandlerc chandlerc commented Dec 9, 2024

Apologies for the large change, I looked for ways to break this up and all of the ones I saw added real complexity. This change focuses on the option's prefixed names and the array of prefixes. These are present in every option and the dominant source of dynamic relocations for PIE or PIC users of LLVM and Clang tooling. In some cases, 100s or 1000s of them for the Clang driver which has a huge number of options.

This PR addresses this by building a string table and a prefixes table that can be referenced with indices rather than pointers that require dynamic relocations. This removes almost 7k dynmaic relocations from the clang binary, roughly 8% of the remaining dynmaic relocations outside of vtables. For busy-boxing use cases where many different option tables are linked into the same binary, the savings add up a bit more.

The string table is a straightforward mechanism, but the prefixes required some subtlety. They are encoded in a Pascal-string fashion with a size followed by a sequence of offsets. This works relatively well for the small realistic prefixes arrays in use.

Lots of code has to change in order to land this though: both all the option library code has to be updated to use the string table and prefixes table, and all the users of the options library have to be updated to correctly instantiate the objects.

Some follow-up patches in the works to provide an abstraction for this style of code, and to start using the same technique for some of the other strings here now that the infrastructure is in place.

Apologies for the large change, I looked for ways to break this up and
all of the ones I saw added real complexity. This change focuses on the
option's prefixed names and the array of prefixes. These are present in
every option and the dominant source of dynamic relocations for PIE or
PIC users of LLVM and Clang tooling. In some cases, 100s or 1000s of
them for the Clang driver which has a huge number of options.

This PR addresses this by building a string table and a prefixes table
that can be referenced with indices rather than pointers that require
dynamic relocations. This removes almost 7k dynmaic relocations from the
`clang` binary, roughly 8% of the remaining dynmaic relocations outside
of vtables. For busy-boxing use cases where many different option tables
are linked into the same binary, the savings add up a bit more.

The string table is a straightforward mechanism, but the prefixes
required some subtlety. They are encoded in a Pascal-string fashion with
a size followed by a sequence of offsets. This works relatively well for
the small realistic prefixes arrays in use.

Lots of code has to change in order to land this though: both all the
option library code has to be updated to use the string table and
prefixes table, and all the users of the options library have to be
updated to correctly instantiate the objects.

Note, I've successfully built and tested `check-{llvm,clang,lld}` with
this change, but had to make edits beyond that. I've ended up needing to
modify LLDB to reflect these changes, but have not been able to build
and test it fully. The relevant binaries build, but `check-lldb`
currently hits unrelated errors for me blocking any progress in checking
those changes.
@chandlerc chandlerc requested a review from dwblaikie December 9, 2024 10:56
@llvmbot llvmbot added clang Clang issues not falling into any other category lld lldb clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' tablegen debuginfo lld:MachO lld:ELF lld:COFF lld:wasm platform:windows llvm:binary-utilities labels Dec 9, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 9, 2024

@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-lld-elf
@llvm/pr-subscribers-lld-wasm
@llvm/pr-subscribers-tablegen
@llvm/pr-subscribers-lld-coff
@llvm/pr-subscribers-llvm-binary-utilities
@llvm/pr-subscribers-lld

@llvm/pr-subscribers-clang

Author: Chandler Carruth (chandlerc)

Changes

Apologies for the large change, I looked for ways to break this up and all of the ones I saw added real complexity. This change focuses on the option's prefixed names and the array of prefixes. These are present in every option and the dominant source of dynamic relocations for PIE or PIC users of LLVM and Clang tooling. In some cases, 100s or 1000s of them for the Clang driver which has a huge number of options.

This PR addresses this by building a string table and a prefixes table that can be referenced with indices rather than pointers that require dynamic relocations. This removes almost 7k dynmaic relocations from the clang binary, roughly 8% of the remaining dynmaic relocations outside of vtables. For busy-boxing use cases where many different option tables are linked into the same binary, the savings add up a bit more.

The string table is a straightforward mechanism, but the prefixes required some subtlety. They are encoded in a Pascal-string fashion with a size followed by a sequence of offsets. This works relatively well for the small realistic prefixes arrays in use.

Lots of code has to change in order to land this though: both all the option library code has to be updated to use the string table and prefixes table, and all the users of the options library have to be updated to correctly instantiate the objects.

Note, I've successfully built and tested check-{llvm,clang,lld} with this change, but had to make edits beyond that. I've ended up needing to modify LLDB to reflect these changes, but have not been able to build and test it fully. The relevant binaries build, but check-lldb currently hits unrelated errors for me blocking any progress in checking those changes.


Patch is 107.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119198.diff

51 Files Affected:

  • (modified) clang/lib/Driver/DriverOptions.cpp (+11-12)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (+43-20)
  • (modified) clang/tools/clang-installapi/Options.cpp (+12-30)
  • (modified) clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp (+8-6)
  • (modified) clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp (+8-6)
  • (modified) clang/tools/clang-scan-deps/ClangScanDeps.cpp (+8-6)
  • (modified) clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp (+8-6)
  • (modified) lld/COFF/DriverUtils.cpp (+8-6)
  • (modified) lld/ELF/DriverUtils.cpp (+8-6)
  • (modified) lld/MachO/DriverUtils.cpp (+8-6)
  • (modified) lld/MinGW/Driver.cpp (+9-7)
  • (modified) lld/wasm/Driver.cpp (+8-7)
  • (modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (+6-2)
  • (modified) lldb/tools/driver/Driver.cpp (+8-6)
  • (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+9-6)
  • (modified) lldb/tools/lldb-server/lldb-gdbserver.cpp (+8-6)
  • (modified) llvm/include/llvm/Option/OptTable.h (+100-44)
  • (modified) llvm/include/llvm/Option/Option.h (+8-6)
  • (modified) llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp (+11-13)
  • (modified) llvm/lib/Option/OptTable.cpp (+91-64)
  • (modified) llvm/lib/Option/Option.cpp (+6-3)
  • (modified) llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp (+9-6)
  • (modified) llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp (+9-6)
  • (modified) llvm/tools/dsymutil/dsymutil.cpp (+8-6)
  • (modified) llvm/tools/llvm-cgdata/llvm-cgdata.cpp (+8-6)
  • (modified) llvm/tools/llvm-cvtres/llvm-cvtres.cpp (+9-6)
  • (modified) llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp (+8-6)
  • (modified) llvm/tools/llvm-debuginfod-find/llvm-debuginfod-find.cpp (+8-6)
  • (modified) llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp (+8-6)
  • (modified) llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp (+8-6)
  • (modified) llvm/tools/llvm-dwp/llvm-dwp.cpp (+8-6)
  • (modified) llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp (+8-6)
  • (modified) llvm/tools/llvm-ifs/llvm-ifs.cpp (+8-6)
  • (modified) llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp (+8-6)
  • (modified) llvm/tools/llvm-lipo/llvm-lipo.cpp (+9-6)
  • (modified) llvm/tools/llvm-ml/llvm-ml.cpp (+9-6)
  • (modified) llvm/tools/llvm-mt/llvm-mt.cpp (+9-6)
  • (modified) llvm/tools/llvm-nm/llvm-nm.cpp (+8-6)
  • (modified) llvm/tools/llvm-objcopy/ObjcopyOptions.cpp (+35-24)
  • (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+22-17)
  • (modified) llvm/tools/llvm-rc/llvm-rc.cpp (+20-12)
  • (modified) llvm/tools/llvm-readobj/llvm-readobj.cpp (+8-6)
  • (modified) llvm/tools/llvm-readtapi/llvm-readtapi.cpp (+8-6)
  • (modified) llvm/tools/llvm-size/llvm-size.cpp (+10-6)
  • (modified) llvm/tools/llvm-strings/llvm-strings.cpp (+8-6)
  • (modified) llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp (+8-6)
  • (modified) llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp (+8-6)
  • (modified) llvm/tools/sancov/sancov.cpp (+8-6)
  • (modified) llvm/unittests/Option/OptionMarshallingTest.cpp (+16-8)
  • (modified) llvm/unittests/Option/OptionParsingTest.cpp (+12-13)
  • (modified) llvm/utils/TableGen/OptionParserEmitter.cpp (+76-39)
diff --git a/clang/lib/Driver/DriverOptions.cpp b/clang/lib/Driver/DriverOptions.cpp
index 053e7f1c6404fe..cde1f8989935b0 100644
--- a/clang/lib/Driver/DriverOptions.cpp
+++ b/clang/lib/Driver/DriverOptions.cpp
@@ -14,24 +14,21 @@ using namespace clang::driver;
 using namespace clang::driver::options;
 using namespace llvm::opt;
 
+#define OPTTABLE_STR_TABLE_CODE
+#include "clang/Driver/Options.inc"
+#undef OPTTABLE_STR_TABLE_CODE
+
 #define OPTTABLE_VALUES_CODE
 #include "clang/Driver/Options.inc"
 #undef OPTTABLE_VALUES_CODE
 
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr llvm::StringLiteral NAME##_init[] = VALUE;                  \
-  static constexpr llvm::ArrayRef<llvm::StringLiteral> NAME(                   \
-      NAME##_init, std::size(NAME##_init) - 1);
+#define OPTTABLE_PREFIXES_TABLE_CODE
 #include "clang/Driver/Options.inc"
-#undef PREFIX
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
-static constexpr const llvm::StringLiteral PrefixTable_init[] =
-#define PREFIX_UNION(VALUES) VALUES
+#define OPTTABLE_PREFIXES_UNION_CODE
 #include "clang/Driver/Options.inc"
-#undef PREFIX_UNION
-    ;
-static constexpr const llvm::ArrayRef<llvm::StringLiteral>
-    PrefixTable(PrefixTable_init, std::size(PrefixTable_init) - 1);
+#undef OPTTABLE_PREFIXES_UNION_CODE
 
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -43,7 +40,9 @@ namespace {
 
 class DriverOptTable : public PrecomputedOptTable {
 public:
-  DriverOptTable() : PrecomputedOptTable(InfoTable, PrefixTable) {}
+  DriverOptTable()
+      : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable,
+                            OptionPrefixesUnion) {}
 };
 }
 
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 98136b7a455d9c..23906d5c06d380 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -277,6 +277,14 @@ CowCompilerInvocation::getMutPreprocessorOutputOpts() {
 
 using ArgumentConsumer = CompilerInvocation::ArgumentConsumer;
 
+#define OPTTABLE_STR_TABLE_CODE
+#include "clang/Driver/Options.inc"
+#undef OPTTABLE_STR_TABLE_CODE
+
+static llvm::StringRef lookupStrInTable(unsigned Offset) {
+  return &OptionStrTable[Offset];
+}
+
 #define SIMPLE_ENUM_VALUE_TABLE
 #include "clang/Driver/Options.inc"
 #undef SIMPLE_ENUM_VALUE_TABLE
@@ -303,6 +311,11 @@ static std::optional<bool> normalizeSimpleNegativeFlag(OptSpecifier Opt,
 /// denormalizeSimpleFlags never looks at it. Avoid bloating compile-time with
 /// unnecessary template instantiations and just ignore it with a variadic
 /// argument.
+static void denormalizeSimpleFlag(ArgumentConsumer Consumer,
+                                  unsigned SpellingOffset, Option::OptionClass,
+                                  unsigned, /*T*/...) {
+  Consumer(lookupStrInTable(SpellingOffset));
+}
 static void denormalizeSimpleFlag(ArgumentConsumer Consumer,
                                   const Twine &Spelling, Option::OptionClass,
                                   unsigned, /*T*/...) {
@@ -343,10 +356,10 @@ static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
 }
 
 static auto makeBooleanOptionDenormalizer(bool Value) {
-  return [Value](ArgumentConsumer Consumer, const Twine &Spelling,
+  return [Value](ArgumentConsumer Consumer, unsigned SpellingOffset,
                  Option::OptionClass, unsigned, bool KeyPath) {
     if (KeyPath == Value)
-      Consumer(Spelling);
+      Consumer(lookupStrInTable(SpellingOffset));
   };
 }
 
@@ -371,6 +384,14 @@ static void denormalizeStringImpl(ArgumentConsumer Consumer,
   }
 }
 
+template <typename T>
+static void
+denormalizeString(ArgumentConsumer Consumer, unsigned SpellingOffset,
+                  Option::OptionClass OptClass, unsigned TableIndex, T Value) {
+  denormalizeStringImpl(Consumer, lookupStrInTable(SpellingOffset), OptClass,
+                        TableIndex, Twine(Value));
+}
+
 template <typename T>
 static void denormalizeString(ArgumentConsumer Consumer, const Twine &Spelling,
                               Option::OptionClass OptClass, unsigned TableIndex,
@@ -417,14 +438,14 @@ static std::optional<unsigned> normalizeSimpleEnum(OptSpecifier Opt,
 }
 
 static void denormalizeSimpleEnumImpl(ArgumentConsumer Consumer,
-                                      const Twine &Spelling,
+                                      unsigned SpellingOffset,
                                       Option::OptionClass OptClass,
                                       unsigned TableIndex, unsigned Value) {
   assert(TableIndex < SimpleEnumValueTablesSize);
   const SimpleEnumValueTable &Table = SimpleEnumValueTables[TableIndex];
   if (auto MaybeEnumVal = findValueTableByValue(Table, Value)) {
-    denormalizeString(Consumer, Spelling, OptClass, TableIndex,
-                      MaybeEnumVal->Name);
+    denormalizeString(Consumer, lookupStrInTable(SpellingOffset), OptClass,
+                      TableIndex, MaybeEnumVal->Name);
   } else {
     llvm_unreachable("The simple enum value was not correctly defined in "
                      "the tablegen option description");
@@ -433,11 +454,11 @@ static void denormalizeSimpleEnumImpl(ArgumentConsumer Consumer,
 
 template <typename T>
 static void denormalizeSimpleEnum(ArgumentConsumer Consumer,
-                                  const Twine &Spelling,
+                                  unsigned SpellingOffset,
                                   Option::OptionClass OptClass,
                                   unsigned TableIndex, T Value) {
-  return denormalizeSimpleEnumImpl(Consumer, Spelling, OptClass, TableIndex,
-                                   static_cast<unsigned>(Value));
+  return denormalizeSimpleEnumImpl(Consumer, SpellingOffset, OptClass,
+                                   TableIndex, static_cast<unsigned>(Value));
 }
 
 static std::optional<std::string> normalizeString(OptSpecifier Opt,
@@ -473,7 +494,7 @@ normalizeStringVector(OptSpecifier Opt, int, const ArgList &Args,
 }
 
 static void denormalizeStringVector(ArgumentConsumer Consumer,
-                                    const Twine &Spelling,
+                                    unsigned SpellingOffset,
                                     Option::OptionClass OptClass,
                                     unsigned TableIndex,
                                     const std::vector<std::string> &Values) {
@@ -487,15 +508,16 @@ static void denormalizeStringVector(ArgumentConsumer Consumer,
         CommaJoinedValue.append(Value);
       }
     }
-    denormalizeString(Consumer, Spelling, Option::OptionClass::JoinedClass,
-                      TableIndex, CommaJoinedValue);
+    denormalizeString(Consumer, SpellingOffset,
+                      Option::OptionClass::JoinedClass, TableIndex,
+                      CommaJoinedValue);
     break;
   }
   case Option::JoinedClass:
   case Option::SeparateClass:
   case Option::JoinedOrSeparateClass:
     for (const std::string &Value : Values)
-      denormalizeString(Consumer, Spelling, OptClass, TableIndex, Value);
+      denormalizeString(Consumer, SpellingOffset, OptClass, TableIndex, Value);
     break;
   default:
     llvm_unreachable("Cannot denormalize an option with option class "
@@ -532,10 +554,11 @@ static T extractMaskValue(T KeyPath) {
 }
 
 #define PARSE_OPTION_WITH_MARSHALLING(                                         \
-    ARGS, DIAGS, PREFIX_TYPE, SPELLING, ID, KIND, GROUP, ALIAS, ALIASARGS,     \
-    FLAGS, VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR, VALUES, \
-    SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, IMPLIED_CHECK,          \
-    IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)   \
+    ARGS, DIAGS, PREFIX_TYPE, SPELLING_OFFSET, ID, KIND, GROUP, ALIAS,         \
+    ALIASARGS, FLAGS, VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS,       \
+    METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,        \
+    IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, \
+    TABLE_INDEX)                                                               \
   if ((VISIBILITY) & options::CC1Option) {                                     \
     KEYPATH = MERGER(KEYPATH, DEFAULT_VALUE);                                  \
     if (IMPLIED_CHECK)                                                         \
@@ -549,8 +572,8 @@ static T extractMaskValue(T KeyPath) {
 // Capture the extracted value as a lambda argument to avoid potential issues
 // with lifetime extension of the reference.
 #define GENERATE_OPTION_WITH_MARSHALLING(                                      \
-    CONSUMER, PREFIX_TYPE, SPELLING, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \
-    VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR, VALUES,        \
+    CONSUMER, PREFIX_TYPE, SPELLING_OFFSET, ID, KIND, GROUP, ALIAS, ALIASARGS, \
+    FLAGS, VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR, VALUES, \
     SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, IMPLIED_CHECK,          \
     IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)   \
   if ((VISIBILITY) & options::CC1Option) {                                     \
@@ -559,8 +582,8 @@ static T extractMaskValue(T KeyPath) {
           (Extracted !=                                                        \
            static_cast<decltype(KEYPATH)>((IMPLIED_CHECK) ? (IMPLIED_VALUE)    \
                                                           : (DEFAULT_VALUE)))) \
-        DENORMALIZER(CONSUMER, SPELLING, Option::KIND##Class, TABLE_INDEX,     \
-                     Extracted);                                               \
+        DENORMALIZER(CONSUMER, SPELLING_OFFSET, Option::KIND##Class,           \
+                     TABLE_INDEX, Extracted);                                  \
     }(EXTRACTOR(KEYPATH));                                                     \
   }
 
diff --git a/clang/tools/clang-installapi/Options.cpp b/clang/tools/clang-installapi/Options.cpp
index 3fa79636de5d75..8a2c3463189fa9 100644
--- a/clang/tools/clang-installapi/Options.cpp
+++ b/clang/tools/clang-installapi/Options.cpp
@@ -31,41 +31,21 @@ namespace drv = clang::driver::options;
 namespace clang {
 namespace installapi {
 
-/// Create prefix string literals used in InstallAPIOpts.td.
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr llvm::StringLiteral NAME##_init[] = VALUE;                  \
-  static constexpr llvm::ArrayRef<llvm::StringLiteral> NAME(                   \
-      NAME##_init, std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "InstallAPIOpts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
 
-static constexpr const llvm::StringLiteral PrefixTable_init[] =
-#define PREFIX_UNION(VALUES) VALUES
+#define OPTTABLE_PREFIXES_TABLE_CODE
 #include "InstallAPIOpts.inc"
-#undef PREFIX_UNION
-    ;
-static constexpr const ArrayRef<StringLiteral>
-    PrefixTable(PrefixTable_init, std::size(PrefixTable_init) - 1);
+#undef OPTTABLE_PREFIXES_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_UNION_CODE
+#include "InstallAPIOpts.inc"
+#undef OPTTABLE_PREFIXES_UNION_CODE
 
 /// Create table mapping all options defined in InstallAPIOpts.td.
 static constexpr OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS,         \
-               VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR,     \
-               VALUES)                                                         \
-  {PREFIX,                                                                     \
-   NAME,                                                                       \
-   HELPTEXT,                                                                   \
-   HELPTEXTSFORVARIANTS,                                                       \
-   METAVAR,                                                                    \
-   OPT_##ID,                                                                   \
-   Option::KIND##Class,                                                        \
-   PARAM,                                                                      \
-   FLAGS,                                                                      \
-   VISIBILITY,                                                                 \
-   OPT_##GROUP,                                                                \
-   OPT_##ALIAS,                                                                \
-   ALIASARGS,                                                                  \
-   VALUES},
+#define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
 #include "InstallAPIOpts.inc"
 #undef OPTION
 };
@@ -75,7 +55,9 @@ namespace {
 /// \brief Create OptTable class for parsing actual command line arguments.
 class DriverOptTable : public opt::PrecomputedOptTable {
 public:
-  DriverOptTable() : PrecomputedOptTable(InfoTable, PrefixTable) {}
+  DriverOptTable()
+      : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable,
+                            OptionPrefixesUnion) {}
 };
 
 } // end anonymous namespace.
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index ebafd7eb7774ec..fae32a3503c185 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -174,12 +174,13 @@ enum ID {
 #undef OPTION
 };
 
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr StringLiteral NAME##_init[] = VALUE;                        \
-  static constexpr ArrayRef<StringLiteral> NAME(NAME##_init,                   \
-                                                std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "LinkerWrapperOpts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_TABLE_CODE
+#include "LinkerWrapperOpts.inc"
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -189,7 +190,8 @@ static constexpr OptTable::Info InfoTable[] = {
 
 class WrapperOptTable : public opt::GenericOptTable {
 public:
-  WrapperOptTable() : opt::GenericOptTable(InfoTable) {}
+  WrapperOptTable()
+      : opt::GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable) {}
 };
 
 const OptTable &getOptTable() {
diff --git a/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp b/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
index bc191afdca739d..faf73a7c2f1938 100644
--- a/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
+++ b/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
@@ -109,12 +109,13 @@ enum ID {
 #undef OPTION
 };
 
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr StringLiteral NAME##_init[] = VALUE;                        \
-  static constexpr ArrayRef<StringLiteral> NAME(NAME##_init,                   \
-                                                std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "NVLinkOpts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_TABLE_CODE
+#include "NVLinkOpts.inc"
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -124,7 +125,8 @@ static constexpr OptTable::Info InfoTable[] = {
 
 class WrapperOptTable : public opt::GenericOptTable {
 public:
-  WrapperOptTable() : opt::GenericOptTable(InfoTable) {}
+  WrapperOptTable()
+      : opt::GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable) {}
 };
 
 const OptTable &getOptTable() {
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 58b56dcfd3bece..bd36181fca3f31 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -50,12 +50,13 @@ enum ID {
 #undef OPTION
 };
 
-#define PREFIX(NAME, VALUE)                                                    \
-  constexpr llvm::StringLiteral NAME##_init[] = VALUE;                         \
-  constexpr llvm::ArrayRef<llvm::StringLiteral> NAME(                          \
-      NAME##_init, std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "Opts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_TABLE_CODE
+#include "Opts.inc"
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 const llvm::opt::OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -65,7 +66,8 @@ const llvm::opt::OptTable::Info InfoTable[] = {
 
 class ScanDepsOptTable : public llvm::opt::GenericOptTable {
 public:
-  ScanDepsOptTable() : GenericOptTable(InfoTable) {
+  ScanDepsOptTable()
+      : GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable) {
     setGroupedShortOptions(true);
   }
 };
diff --git a/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp b/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
index 076458a275d986..2bcb3757d49d08 100644
--- a/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
+++ b/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
@@ -88,12 +88,13 @@ enum ID {
 #undef OPTION
 };
 
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr StringLiteral NAME##_init[] = VALUE;                        \
-  static constexpr ArrayRef<StringLiteral> NAME(NAME##_init,                   \
-                                                std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "SYCLLinkOpts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_TABLE_CODE
+#include "SYCLLinkOpts.inc"
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -103,7 +104,8 @@ static constexpr OptTable::Info InfoTable[] = {
 
 class LinkerOptTable : public opt::GenericOptTable {
 public:
-  LinkerOptTable() : opt::GenericOptTable(InfoTable) {}
+  LinkerOptTable()
+      : opt::GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable) {}
 };
 
 const OptTable &getOptTable() {
diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp
index c1b3272a1f49ea..1148be09fb10cc 100644
--- a/lld/COFF/DriverUtils.cpp
+++ b/lld/COFF/DriverUtils.cpp
@@ -845,13 +845,14 @@ MemoryBufferRef LinkerDriver::convertResToCOFF(ArrayRef<MemoryBufferRef> mbs,
 
 // Create OptTable
 
+#define OPTTABLE_STR_TABLE_CODE
+#include "Options.inc"
+#undef OPTTABLE_STR_TABLE_CODE
+
 // Create prefix string literals used in Options.td
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr llvm::StringLiteral NAME##_init[] = VALUE;                  \
-  static constexpr llvm::ArrayRef<llvm::StringLiteral> NAME(                   \
-      NAME##_init, std::size(NAME##_init) - 1);
+#define OPTTABLE_PREFIXES_TABLE_CODE
 #include "Options.inc"
-#undef PREFIX
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 // Create table mapping all options defined in Options.td
 static constexpr llvm::opt::OptTable::Info infoTable[] = {
@@ -860,7 +861,8 @@ static constexpr llvm::opt::OptTable::Info infoTable[] = {
 #undef OPTION
 };
 
-COFFOptTable::COFFOptTable() : GenericOptTable(infoTable, true) {}
+COFFOptTable::COFFOptTable()
+    : GenericOptTable(OptionStrTable, OptionPrefixesTable, infoTable, true) {}
 
 // Set color diagnostics according to --color-diagnostics={auto,always,never}
 // or --no-color-diagnostics flags.
diff --git a/lld/ELF/DriverUtils.cpp b/lld/ELF/DriverUtils.cpp
index 4c88723f090d08..6d027c529c19e9 100644
--- a/lld/ELF/DriverUtils.cpp
+++ b/lld/ELF/DriverUtils.cpp
@@ -33,13 +33,14 @@ using namespace lld::elf;
 
 // Create OptTable
 
+#define OPTTABLE_STR_TABLE_CODE
+#include "Options.inc"
+#undef OPTTABLE_STR_TABLE_CODE
+
 // Create prefix string literals used in Options.td
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr StringLiteral NAME##_init[] = VALUE;                        \
-  static constexpr ArrayRef<StringLit...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Dec 9, 2024

@llvm/pr-subscribers-clang-driver

Author: Chandler Carruth (chandlerc)

Changes

Apologies for the large change, I looked for ways to break this up and all of the ones I saw added real complexity. This change focuses on the option's prefixed names and the array of prefixes. These are present in every option and the dominant source of dynamic relocations for PIE or PIC users of LLVM and Clang tooling. In some cases, 100s or 1000s of them for the Clang driver which has a huge number of options.

This PR addresses this by building a string table and a prefixes table that can be referenced with indices rather than pointers that require dynamic relocations. This removes almost 7k dynmaic relocations from the clang binary, roughly 8% of the remaining dynmaic relocations outside of vtables. For busy-boxing use cases where many different option tables are linked into the same binary, the savings add up a bit more.

The string table is a straightforward mechanism, but the prefixes required some subtlety. They are encoded in a Pascal-string fashion with a size followed by a sequence of offsets. This works relatively well for the small realistic prefixes arrays in use.

Lots of code has to change in order to land this though: both all the option library code has to be updated to use the string table and prefixes table, and all the users of the options library have to be updated to correctly instantiate the objects.

Note, I've successfully built and tested check-{llvm,clang,lld} with this change, but had to make edits beyond that. I've ended up needing to modify LLDB to reflect these changes, but have not been able to build and test it fully. The relevant binaries build, but check-lldb currently hits unrelated errors for me blocking any progress in checking those changes.


Patch is 107.31 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119198.diff

51 Files Affected:

  • (modified) clang/lib/Driver/DriverOptions.cpp (+11-12)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (+43-20)
  • (modified) clang/tools/clang-installapi/Options.cpp (+12-30)
  • (modified) clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp (+8-6)
  • (modified) clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp (+8-6)
  • (modified) clang/tools/clang-scan-deps/ClangScanDeps.cpp (+8-6)
  • (modified) clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp (+8-6)
  • (modified) lld/COFF/DriverUtils.cpp (+8-6)
  • (modified) lld/ELF/DriverUtils.cpp (+8-6)
  • (modified) lld/MachO/DriverUtils.cpp (+8-6)
  • (modified) lld/MinGW/Driver.cpp (+9-7)
  • (modified) lld/wasm/Driver.cpp (+8-7)
  • (modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (+6-2)
  • (modified) lldb/tools/driver/Driver.cpp (+8-6)
  • (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+9-6)
  • (modified) lldb/tools/lldb-server/lldb-gdbserver.cpp (+8-6)
  • (modified) llvm/include/llvm/Option/OptTable.h (+100-44)
  • (modified) llvm/include/llvm/Option/Option.h (+8-6)
  • (modified) llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp (+11-13)
  • (modified) llvm/lib/Option/OptTable.cpp (+91-64)
  • (modified) llvm/lib/Option/Option.cpp (+6-3)
  • (modified) llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp (+9-6)
  • (modified) llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp (+9-6)
  • (modified) llvm/tools/dsymutil/dsymutil.cpp (+8-6)
  • (modified) llvm/tools/llvm-cgdata/llvm-cgdata.cpp (+8-6)
  • (modified) llvm/tools/llvm-cvtres/llvm-cvtres.cpp (+9-6)
  • (modified) llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp (+8-6)
  • (modified) llvm/tools/llvm-debuginfod-find/llvm-debuginfod-find.cpp (+8-6)
  • (modified) llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp (+8-6)
  • (modified) llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp (+8-6)
  • (modified) llvm/tools/llvm-dwp/llvm-dwp.cpp (+8-6)
  • (modified) llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp (+8-6)
  • (modified) llvm/tools/llvm-ifs/llvm-ifs.cpp (+8-6)
  • (modified) llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp (+8-6)
  • (modified) llvm/tools/llvm-lipo/llvm-lipo.cpp (+9-6)
  • (modified) llvm/tools/llvm-ml/llvm-ml.cpp (+9-6)
  • (modified) llvm/tools/llvm-mt/llvm-mt.cpp (+9-6)
  • (modified) llvm/tools/llvm-nm/llvm-nm.cpp (+8-6)
  • (modified) llvm/tools/llvm-objcopy/ObjcopyOptions.cpp (+35-24)
  • (modified) llvm/tools/llvm-objdump/llvm-objdump.cpp (+22-17)
  • (modified) llvm/tools/llvm-rc/llvm-rc.cpp (+20-12)
  • (modified) llvm/tools/llvm-readobj/llvm-readobj.cpp (+8-6)
  • (modified) llvm/tools/llvm-readtapi/llvm-readtapi.cpp (+8-6)
  • (modified) llvm/tools/llvm-size/llvm-size.cpp (+10-6)
  • (modified) llvm/tools/llvm-strings/llvm-strings.cpp (+8-6)
  • (modified) llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp (+8-6)
  • (modified) llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp (+8-6)
  • (modified) llvm/tools/sancov/sancov.cpp (+8-6)
  • (modified) llvm/unittests/Option/OptionMarshallingTest.cpp (+16-8)
  • (modified) llvm/unittests/Option/OptionParsingTest.cpp (+12-13)
  • (modified) llvm/utils/TableGen/OptionParserEmitter.cpp (+76-39)
diff --git a/clang/lib/Driver/DriverOptions.cpp b/clang/lib/Driver/DriverOptions.cpp
index 053e7f1c6404fe..cde1f8989935b0 100644
--- a/clang/lib/Driver/DriverOptions.cpp
+++ b/clang/lib/Driver/DriverOptions.cpp
@@ -14,24 +14,21 @@ using namespace clang::driver;
 using namespace clang::driver::options;
 using namespace llvm::opt;
 
+#define OPTTABLE_STR_TABLE_CODE
+#include "clang/Driver/Options.inc"
+#undef OPTTABLE_STR_TABLE_CODE
+
 #define OPTTABLE_VALUES_CODE
 #include "clang/Driver/Options.inc"
 #undef OPTTABLE_VALUES_CODE
 
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr llvm::StringLiteral NAME##_init[] = VALUE;                  \
-  static constexpr llvm::ArrayRef<llvm::StringLiteral> NAME(                   \
-      NAME##_init, std::size(NAME##_init) - 1);
+#define OPTTABLE_PREFIXES_TABLE_CODE
 #include "clang/Driver/Options.inc"
-#undef PREFIX
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
-static constexpr const llvm::StringLiteral PrefixTable_init[] =
-#define PREFIX_UNION(VALUES) VALUES
+#define OPTTABLE_PREFIXES_UNION_CODE
 #include "clang/Driver/Options.inc"
-#undef PREFIX_UNION
-    ;
-static constexpr const llvm::ArrayRef<llvm::StringLiteral>
-    PrefixTable(PrefixTable_init, std::size(PrefixTable_init) - 1);
+#undef OPTTABLE_PREFIXES_UNION_CODE
 
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -43,7 +40,9 @@ namespace {
 
 class DriverOptTable : public PrecomputedOptTable {
 public:
-  DriverOptTable() : PrecomputedOptTable(InfoTable, PrefixTable) {}
+  DriverOptTable()
+      : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable,
+                            OptionPrefixesUnion) {}
 };
 }
 
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 98136b7a455d9c..23906d5c06d380 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -277,6 +277,14 @@ CowCompilerInvocation::getMutPreprocessorOutputOpts() {
 
 using ArgumentConsumer = CompilerInvocation::ArgumentConsumer;
 
+#define OPTTABLE_STR_TABLE_CODE
+#include "clang/Driver/Options.inc"
+#undef OPTTABLE_STR_TABLE_CODE
+
+static llvm::StringRef lookupStrInTable(unsigned Offset) {
+  return &OptionStrTable[Offset];
+}
+
 #define SIMPLE_ENUM_VALUE_TABLE
 #include "clang/Driver/Options.inc"
 #undef SIMPLE_ENUM_VALUE_TABLE
@@ -303,6 +311,11 @@ static std::optional<bool> normalizeSimpleNegativeFlag(OptSpecifier Opt,
 /// denormalizeSimpleFlags never looks at it. Avoid bloating compile-time with
 /// unnecessary template instantiations and just ignore it with a variadic
 /// argument.
+static void denormalizeSimpleFlag(ArgumentConsumer Consumer,
+                                  unsigned SpellingOffset, Option::OptionClass,
+                                  unsigned, /*T*/...) {
+  Consumer(lookupStrInTable(SpellingOffset));
+}
 static void denormalizeSimpleFlag(ArgumentConsumer Consumer,
                                   const Twine &Spelling, Option::OptionClass,
                                   unsigned, /*T*/...) {
@@ -343,10 +356,10 @@ static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
 }
 
 static auto makeBooleanOptionDenormalizer(bool Value) {
-  return [Value](ArgumentConsumer Consumer, const Twine &Spelling,
+  return [Value](ArgumentConsumer Consumer, unsigned SpellingOffset,
                  Option::OptionClass, unsigned, bool KeyPath) {
     if (KeyPath == Value)
-      Consumer(Spelling);
+      Consumer(lookupStrInTable(SpellingOffset));
   };
 }
 
@@ -371,6 +384,14 @@ static void denormalizeStringImpl(ArgumentConsumer Consumer,
   }
 }
 
+template <typename T>
+static void
+denormalizeString(ArgumentConsumer Consumer, unsigned SpellingOffset,
+                  Option::OptionClass OptClass, unsigned TableIndex, T Value) {
+  denormalizeStringImpl(Consumer, lookupStrInTable(SpellingOffset), OptClass,
+                        TableIndex, Twine(Value));
+}
+
 template <typename T>
 static void denormalizeString(ArgumentConsumer Consumer, const Twine &Spelling,
                               Option::OptionClass OptClass, unsigned TableIndex,
@@ -417,14 +438,14 @@ static std::optional<unsigned> normalizeSimpleEnum(OptSpecifier Opt,
 }
 
 static void denormalizeSimpleEnumImpl(ArgumentConsumer Consumer,
-                                      const Twine &Spelling,
+                                      unsigned SpellingOffset,
                                       Option::OptionClass OptClass,
                                       unsigned TableIndex, unsigned Value) {
   assert(TableIndex < SimpleEnumValueTablesSize);
   const SimpleEnumValueTable &Table = SimpleEnumValueTables[TableIndex];
   if (auto MaybeEnumVal = findValueTableByValue(Table, Value)) {
-    denormalizeString(Consumer, Spelling, OptClass, TableIndex,
-                      MaybeEnumVal->Name);
+    denormalizeString(Consumer, lookupStrInTable(SpellingOffset), OptClass,
+                      TableIndex, MaybeEnumVal->Name);
   } else {
     llvm_unreachable("The simple enum value was not correctly defined in "
                      "the tablegen option description");
@@ -433,11 +454,11 @@ static void denormalizeSimpleEnumImpl(ArgumentConsumer Consumer,
 
 template <typename T>
 static void denormalizeSimpleEnum(ArgumentConsumer Consumer,
-                                  const Twine &Spelling,
+                                  unsigned SpellingOffset,
                                   Option::OptionClass OptClass,
                                   unsigned TableIndex, T Value) {
-  return denormalizeSimpleEnumImpl(Consumer, Spelling, OptClass, TableIndex,
-                                   static_cast<unsigned>(Value));
+  return denormalizeSimpleEnumImpl(Consumer, SpellingOffset, OptClass,
+                                   TableIndex, static_cast<unsigned>(Value));
 }
 
 static std::optional<std::string> normalizeString(OptSpecifier Opt,
@@ -473,7 +494,7 @@ normalizeStringVector(OptSpecifier Opt, int, const ArgList &Args,
 }
 
 static void denormalizeStringVector(ArgumentConsumer Consumer,
-                                    const Twine &Spelling,
+                                    unsigned SpellingOffset,
                                     Option::OptionClass OptClass,
                                     unsigned TableIndex,
                                     const std::vector<std::string> &Values) {
@@ -487,15 +508,16 @@ static void denormalizeStringVector(ArgumentConsumer Consumer,
         CommaJoinedValue.append(Value);
       }
     }
-    denormalizeString(Consumer, Spelling, Option::OptionClass::JoinedClass,
-                      TableIndex, CommaJoinedValue);
+    denormalizeString(Consumer, SpellingOffset,
+                      Option::OptionClass::JoinedClass, TableIndex,
+                      CommaJoinedValue);
     break;
   }
   case Option::JoinedClass:
   case Option::SeparateClass:
   case Option::JoinedOrSeparateClass:
     for (const std::string &Value : Values)
-      denormalizeString(Consumer, Spelling, OptClass, TableIndex, Value);
+      denormalizeString(Consumer, SpellingOffset, OptClass, TableIndex, Value);
     break;
   default:
     llvm_unreachable("Cannot denormalize an option with option class "
@@ -532,10 +554,11 @@ static T extractMaskValue(T KeyPath) {
 }
 
 #define PARSE_OPTION_WITH_MARSHALLING(                                         \
-    ARGS, DIAGS, PREFIX_TYPE, SPELLING, ID, KIND, GROUP, ALIAS, ALIASARGS,     \
-    FLAGS, VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR, VALUES, \
-    SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, IMPLIED_CHECK,          \
-    IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)   \
+    ARGS, DIAGS, PREFIX_TYPE, SPELLING_OFFSET, ID, KIND, GROUP, ALIAS,         \
+    ALIASARGS, FLAGS, VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS,       \
+    METAVAR, VALUES, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,        \
+    IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, \
+    TABLE_INDEX)                                                               \
   if ((VISIBILITY) & options::CC1Option) {                                     \
     KEYPATH = MERGER(KEYPATH, DEFAULT_VALUE);                                  \
     if (IMPLIED_CHECK)                                                         \
@@ -549,8 +572,8 @@ static T extractMaskValue(T KeyPath) {
 // Capture the extracted value as a lambda argument to avoid potential issues
 // with lifetime extension of the reference.
 #define GENERATE_OPTION_WITH_MARSHALLING(                                      \
-    CONSUMER, PREFIX_TYPE, SPELLING, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, \
-    VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR, VALUES,        \
+    CONSUMER, PREFIX_TYPE, SPELLING_OFFSET, ID, KIND, GROUP, ALIAS, ALIASARGS, \
+    FLAGS, VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR, VALUES, \
     SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, IMPLIED_CHECK,          \
     IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)   \
   if ((VISIBILITY) & options::CC1Option) {                                     \
@@ -559,8 +582,8 @@ static T extractMaskValue(T KeyPath) {
           (Extracted !=                                                        \
            static_cast<decltype(KEYPATH)>((IMPLIED_CHECK) ? (IMPLIED_VALUE)    \
                                                           : (DEFAULT_VALUE)))) \
-        DENORMALIZER(CONSUMER, SPELLING, Option::KIND##Class, TABLE_INDEX,     \
-                     Extracted);                                               \
+        DENORMALIZER(CONSUMER, SPELLING_OFFSET, Option::KIND##Class,           \
+                     TABLE_INDEX, Extracted);                                  \
     }(EXTRACTOR(KEYPATH));                                                     \
   }
 
diff --git a/clang/tools/clang-installapi/Options.cpp b/clang/tools/clang-installapi/Options.cpp
index 3fa79636de5d75..8a2c3463189fa9 100644
--- a/clang/tools/clang-installapi/Options.cpp
+++ b/clang/tools/clang-installapi/Options.cpp
@@ -31,41 +31,21 @@ namespace drv = clang::driver::options;
 namespace clang {
 namespace installapi {
 
-/// Create prefix string literals used in InstallAPIOpts.td.
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr llvm::StringLiteral NAME##_init[] = VALUE;                  \
-  static constexpr llvm::ArrayRef<llvm::StringLiteral> NAME(                   \
-      NAME##_init, std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "InstallAPIOpts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
 
-static constexpr const llvm::StringLiteral PrefixTable_init[] =
-#define PREFIX_UNION(VALUES) VALUES
+#define OPTTABLE_PREFIXES_TABLE_CODE
 #include "InstallAPIOpts.inc"
-#undef PREFIX_UNION
-    ;
-static constexpr const ArrayRef<StringLiteral>
-    PrefixTable(PrefixTable_init, std::size(PrefixTable_init) - 1);
+#undef OPTTABLE_PREFIXES_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_UNION_CODE
+#include "InstallAPIOpts.inc"
+#undef OPTTABLE_PREFIXES_UNION_CODE
 
 /// Create table mapping all options defined in InstallAPIOpts.td.
 static constexpr OptTable::Info InfoTable[] = {
-#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS,         \
-               VISIBILITY, PARAM, HELPTEXT, HELPTEXTSFORVARIANTS, METAVAR,     \
-               VALUES)                                                         \
-  {PREFIX,                                                                     \
-   NAME,                                                                       \
-   HELPTEXT,                                                                   \
-   HELPTEXTSFORVARIANTS,                                                       \
-   METAVAR,                                                                    \
-   OPT_##ID,                                                                   \
-   Option::KIND##Class,                                                        \
-   PARAM,                                                                      \
-   FLAGS,                                                                      \
-   VISIBILITY,                                                                 \
-   OPT_##GROUP,                                                                \
-   OPT_##ALIAS,                                                                \
-   ALIASARGS,                                                                  \
-   VALUES},
+#define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
 #include "InstallAPIOpts.inc"
 #undef OPTION
 };
@@ -75,7 +55,9 @@ namespace {
 /// \brief Create OptTable class for parsing actual command line arguments.
 class DriverOptTable : public opt::PrecomputedOptTable {
 public:
-  DriverOptTable() : PrecomputedOptTable(InfoTable, PrefixTable) {}
+  DriverOptTable()
+      : PrecomputedOptTable(OptionStrTable, OptionPrefixesTable, InfoTable,
+                            OptionPrefixesUnion) {}
 };
 
 } // end anonymous namespace.
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index ebafd7eb7774ec..fae32a3503c185 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -174,12 +174,13 @@ enum ID {
 #undef OPTION
 };
 
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr StringLiteral NAME##_init[] = VALUE;                        \
-  static constexpr ArrayRef<StringLiteral> NAME(NAME##_init,                   \
-                                                std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "LinkerWrapperOpts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_TABLE_CODE
+#include "LinkerWrapperOpts.inc"
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -189,7 +190,8 @@ static constexpr OptTable::Info InfoTable[] = {
 
 class WrapperOptTable : public opt::GenericOptTable {
 public:
-  WrapperOptTable() : opt::GenericOptTable(InfoTable) {}
+  WrapperOptTable()
+      : opt::GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable) {}
 };
 
 const OptTable &getOptTable() {
diff --git a/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp b/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
index bc191afdca739d..faf73a7c2f1938 100644
--- a/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
+++ b/clang/tools/clang-nvlink-wrapper/ClangNVLinkWrapper.cpp
@@ -109,12 +109,13 @@ enum ID {
 #undef OPTION
 };
 
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr StringLiteral NAME##_init[] = VALUE;                        \
-  static constexpr ArrayRef<StringLiteral> NAME(NAME##_init,                   \
-                                                std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "NVLinkOpts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_TABLE_CODE
+#include "NVLinkOpts.inc"
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -124,7 +125,8 @@ static constexpr OptTable::Info InfoTable[] = {
 
 class WrapperOptTable : public opt::GenericOptTable {
 public:
-  WrapperOptTable() : opt::GenericOptTable(InfoTable) {}
+  WrapperOptTable()
+      : opt::GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable) {}
 };
 
 const OptTable &getOptTable() {
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 58b56dcfd3bece..bd36181fca3f31 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -50,12 +50,13 @@ enum ID {
 #undef OPTION
 };
 
-#define PREFIX(NAME, VALUE)                                                    \
-  constexpr llvm::StringLiteral NAME##_init[] = VALUE;                         \
-  constexpr llvm::ArrayRef<llvm::StringLiteral> NAME(                          \
-      NAME##_init, std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "Opts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_TABLE_CODE
+#include "Opts.inc"
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 const llvm::opt::OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -65,7 +66,8 @@ const llvm::opt::OptTable::Info InfoTable[] = {
 
 class ScanDepsOptTable : public llvm::opt::GenericOptTable {
 public:
-  ScanDepsOptTable() : GenericOptTable(InfoTable) {
+  ScanDepsOptTable()
+      : GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable) {
     setGroupedShortOptions(true);
   }
 };
diff --git a/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp b/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
index 076458a275d986..2bcb3757d49d08 100644
--- a/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
+++ b/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
@@ -88,12 +88,13 @@ enum ID {
 #undef OPTION
 };
 
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr StringLiteral NAME##_init[] = VALUE;                        \
-  static constexpr ArrayRef<StringLiteral> NAME(NAME##_init,                   \
-                                                std::size(NAME##_init) - 1);
+#define OPTTABLE_STR_TABLE_CODE
 #include "SYCLLinkOpts.inc"
-#undef PREFIX
+#undef OPTTABLE_STR_TABLE_CODE
+
+#define OPTTABLE_PREFIXES_TABLE_CODE
+#include "SYCLLinkOpts.inc"
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
@@ -103,7 +104,8 @@ static constexpr OptTable::Info InfoTable[] = {
 
 class LinkerOptTable : public opt::GenericOptTable {
 public:
-  LinkerOptTable() : opt::GenericOptTable(InfoTable) {}
+  LinkerOptTable()
+      : opt::GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable) {}
 };
 
 const OptTable &getOptTable() {
diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp
index c1b3272a1f49ea..1148be09fb10cc 100644
--- a/lld/COFF/DriverUtils.cpp
+++ b/lld/COFF/DriverUtils.cpp
@@ -845,13 +845,14 @@ MemoryBufferRef LinkerDriver::convertResToCOFF(ArrayRef<MemoryBufferRef> mbs,
 
 // Create OptTable
 
+#define OPTTABLE_STR_TABLE_CODE
+#include "Options.inc"
+#undef OPTTABLE_STR_TABLE_CODE
+
 // Create prefix string literals used in Options.td
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr llvm::StringLiteral NAME##_init[] = VALUE;                  \
-  static constexpr llvm::ArrayRef<llvm::StringLiteral> NAME(                   \
-      NAME##_init, std::size(NAME##_init) - 1);
+#define OPTTABLE_PREFIXES_TABLE_CODE
 #include "Options.inc"
-#undef PREFIX
+#undef OPTTABLE_PREFIXES_TABLE_CODE
 
 // Create table mapping all options defined in Options.td
 static constexpr llvm::opt::OptTable::Info infoTable[] = {
@@ -860,7 +861,8 @@ static constexpr llvm::opt::OptTable::Info infoTable[] = {
 #undef OPTION
 };
 
-COFFOptTable::COFFOptTable() : GenericOptTable(infoTable, true) {}
+COFFOptTable::COFFOptTable()
+    : GenericOptTable(OptionStrTable, OptionPrefixesTable, infoTable, true) {}
 
 // Set color diagnostics according to --color-diagnostics={auto,always,never}
 // or --no-color-diagnostics flags.
diff --git a/lld/ELF/DriverUtils.cpp b/lld/ELF/DriverUtils.cpp
index 4c88723f090d08..6d027c529c19e9 100644
--- a/lld/ELF/DriverUtils.cpp
+++ b/lld/ELF/DriverUtils.cpp
@@ -33,13 +33,14 @@ using namespace lld::elf;
 
 // Create OptTable
 
+#define OPTTABLE_STR_TABLE_CODE
+#include "Options.inc"
+#undef OPTTABLE_STR_TABLE_CODE
+
 // Create prefix string literals used in Options.td
-#define PREFIX(NAME, VALUE)                                                    \
-  static constexpr StringLiteral NAME##_init[] = VALUE;                        \
-  static constexpr ArrayRef<StringLit...
[truncated]

@DavidSpickett
Copy link
Collaborator

The relevant binaries build, but check-lldb currently hits unrelated errors for me blocking any progress in checking those changes.

ninja check-lldb passes on AArch64 Linux, and given the changes, it's unlikely to fail on other platforms.

@DavidSpickett
Copy link
Collaborator

If you look for:

FAILED: �[0mtools/clang/tools/extra/clangd/CMakeFiles/obj.clangDaemon.dir/CompileCommands.cpp.o 

In the build log that's the failure reason.

All of the tests that reported JSON passed, that's why you have green test reports but failed builds. I will see what I can do about that.

@chandlerc
Copy link
Member Author

If you look for:

FAILED: �[0mtools/clang/tools/extra/clangd/CMakeFiles/obj.clangDaemon.dir/CompileCommands.cpp.o 

In the build log that's the failure reason.

NP, and thanks. Sorry I missed that set of tests. Pushed a fix that makes check-clang-tools clean.

Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think this is ready to land.

@chandlerc
Copy link
Member Author

Thanks for the careful review, merging! (And starting on the follow-ups!)

@chandlerc chandlerc merged commit dd647e3 into llvm:main Dec 11, 2024
10 checks passed
@chandlerc chandlerc deleted the more-strtable branch December 11, 2024 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants