[OptTable] Make ValuesCode initialisation of Options constexpr
authorserge-sans-paille <sguelton@mozilla.com>
Mon, 26 Dec 2022 07:51:47 +0000 (08:51 +0100)
committerserge-sans-paille <sguelton@mozilla.com>
Thu, 12 Jan 2023 11:08:01 +0000 (12:08 +0100)
Current implementation requires a copy of the initialization array to a
vector to be able to modify their Values field.

This is inefficient: it requires a large copy to update a value, while
TableGen has all information to avoid this overwrite.

Modify TableGen to emit the Values code and use it to perform the
initialisation.

The impact on performance is not amazing compared to the actual
compilation, but still noticeable:

https://llvm-compile-time-tracker.com/compare.php?from=d9ab3e82f30d646deff054230b0c742704a1cf26&to=f2b37fb65d5149f70b43d1801beb5239285a2a20&stat=instructions:u

Differential Revision: https://reviews.llvm.org/D140699

clang/include/clang/Driver/Options.td
clang/lib/Driver/DriverOptions.cpp
llvm/include/llvm/Option/OptTable.h
llvm/lib/Option/OptTable.cpp
llvm/utils/TableGen/OptParserEmitter.cpp

index 3fcf9dd..6cee51f 100644 (file)
@@ -4252,7 +4252,7 @@ def std_default_EQ : Joined<["-"], "std-default=">;
 def std_EQ : Joined<["-", "--"], "std=">, Flags<[CC1Option,FlangOption,FC1Option]>,
   Group<CompileOnly_Group>, HelpText<"Language standard to compile for">,
   ValuesCode<[{
-    const char *Values =
+    static constexpr const char VALUES_CODE [] =
     #define LANGSTANDARD(id, name, lang, desc, features) name ","
     #define LANGSTANDARD_ALIAS(id, alias) alias ","
     #include "clang/Basic/LangStandards.def"
@@ -5263,7 +5263,7 @@ def analyzer_stats : Flag<["-"], "analyzer-stats">,
 def analyzer_checker : Separate<["-"], "analyzer-checker">,
   HelpText<"Choose analyzer checkers to enable">,
   ValuesCode<[{
-    const char *Values =
+    static constexpr const char VALUES_CODE [] =
     #define GET_CHECKERS
     #define CHECKER(FULLNAME, CLASS, HT, DOC_URI, IS_HIDDEN)  FULLNAME ","
     #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
index 197fa0a..0a14749 100644 (file)
@@ -16,6 +16,10 @@ using namespace clang::driver;
 using namespace clang::driver::options;
 using namespace llvm::opt;
 
+#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(                   \
@@ -43,16 +47,6 @@ public:
 }
 
 const llvm::opt::OptTable &clang::driver::getDriverOptTable() {
-  static const DriverOptTable *Table = []() {
-    auto Result = std::make_unique<DriverOptTable>();
-    // Options.inc is included in DriverOptions.cpp, and calls OptTable's
-    // addValues function.
-    // Opt is a variable used in the code fragment in Options.inc.
-    OptTable &Opt = *Result;
-#define OPTTABLE_ARG_INIT
-#include "clang/Driver/Options.inc"
-#undef OPTTABLE_ARG_INIT
-    return Result.release();
-  }();
-  return *Table;
+  static DriverOptTable Table;
+  return Table;
 }
index 52354eb..4933116 100644 (file)
@@ -60,7 +60,7 @@ public:
 
 private:
   /// The option information table.
-  std::vector<Info> OptionInfos;
+  ArrayRef<Info> OptionInfos;
   bool IgnoreCase;
   bool GroupedShortOptions = false;
   const char *EnvVar = nullptr;
@@ -174,17 +174,6 @@ public:
                        unsigned FlagsToInclude = 0, unsigned FlagsToExclude = 0,
                        unsigned MinimumLength = 4) const;
 
-  /// Add Values to Option's Values class
-  ///
-  /// \param [in] Option - Prefix + Name of the flag which Values will be
-  ///  changed. For example, "-analyzer-checker".
-  /// \param [in] Values - String of Values seperated by ",", such as
-  ///  "foo, bar..", where foo and bar is the argument which the Option flag
-  ///  takes
-  ///
-  /// \return true in success, and false in fail.
-  bool addValues(StringRef Option, const char *Values);
-
   /// Parse a single argument; returning the new argument and
   /// updating Index.
   ///
index 3b4e439..49d56c0 100644 (file)
@@ -300,17 +300,6 @@ unsigned OptTable::findNearest(StringRef Option, std::string &NearestString,
   return BestDistance;
 }
 
-bool OptTable::addValues(StringRef Option, const char *Values) {
-  for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) {
-    Info &In = OptionInfos[I];
-    if (optionMatches(In, Option)) {
-      In.Values = Values;
-      return true;
-    }
-  }
-  return false;
-}
-
 // Parse a single argument, return the new argument, and update Index. If
 // GroupedShortOptions is true, -a matches "-abc" and the argument in Args will
 // be updated to "-bc". This overload does not support
index 8c57dda..b897843 100644 (file)
@@ -260,6 +260,21 @@ void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
   OS << "#endif // PREFIX\n\n";
 
   OS << "/////////\n";
+  OS << "// ValuesCode\n\n";
+  OS << "#ifdef OPTTABLE_VALUES_CODE\n";
+  for (const Record &R : llvm::make_pointee_range(Opts)) {
+    // The option values, if any;
+    if (!isa<UnsetInit>(R.getValueInit("ValuesCode"))) {
+      assert(isa<UnsetInit>(R.getValueInit("Values")) &&
+             "Cannot choose between Values and ValuesCode");
+      OS << "#define VALUES_CODE " << getOptionName(R) << "_Values\n";
+      OS << R.getValueAsString("ValuesCode") << "\n";
+      OS << "#undef VALUES_CODE\n";
+    }
+  }
+  OS << "#endif\n";
+
+  OS << "/////////\n";
   OS << "// Groups\n\n";
   OS << "#ifdef OPTION\n";
   for (const Record &R : llvm::make_pointee_range(Groups)) {
@@ -388,6 +403,9 @@ void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
     OS << ", ";
     if (!isa<UnsetInit>(R.getValueInit("Values")))
       write_cstring(OS, R.getValueAsString("Values"));
+    else if (!isa<UnsetInit>(R.getValueInit("ValuesCode"))) {
+      OS << getOptionName(R) << "_Values";
+    }
     else
       OS << "nullptr";
   };
@@ -460,29 +478,5 @@ void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
   OS << "\n";
 
   OS << "\n";
-  OS << "#ifdef OPTTABLE_ARG_INIT\n";
-  OS << "//////////\n";
-  OS << "// Option Values\n\n";
-  for (const Record &R : llvm::make_pointee_range(Opts)) {
-    if (isa<UnsetInit>(R.getValueInit("ValuesCode")))
-      continue;
-    OS << "{\n";
-    OS << "bool ValuesWereAdded;\n";
-    OS << R.getValueAsString("ValuesCode");
-    OS << "\n";
-    for (StringRef Prefix : R.getValueAsListOfStrings("Prefixes")) {
-      OS << "ValuesWereAdded = Opt.addValues(";
-      std::string S(Prefix);
-      S += R.getValueAsString("Name");
-      write_cstring(OS, S);
-      OS << ", Values);\n";
-      OS << "(void)ValuesWereAdded;\n";
-      OS << "assert(ValuesWereAdded && \"Couldn't add values to "
-            "OptTable!\");\n";
-    }
-    OS << "}\n";
-  }
-  OS << "\n";
-  OS << "#endif // OPTTABLE_ARG_INIT\n";
 }
 } // end namespace llvm