serge-sans-paille created this revision.
serge-sans-paille added reviewers: aaron.ballman, nikic.
Herald added a subscriber: hiraditya.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D140699
Files:
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: llvm/utils/TableGen/OptParserEmitter.cpp
===================================================================
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -259,6 +259,21 @@
OS << "#undef COMMA\n";
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";
@@ -388,6 +403,9 @@
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 @@
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
Index: llvm/lib/Option/OptTable.cpp
===================================================================
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -300,17 +300,6 @@
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: llvm/include/llvm/Option/OptTable.h
===================================================================
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -60,7 +60,7 @@
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 @@
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: clang/lib/Driver/DriverOptions.cpp
===================================================================
--- clang/lib/Driver/DriverOptions.cpp
+++ clang/lib/Driver/DriverOptions.cpp
@@ -16,6 +16,10 @@
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 @@
}
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: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4228,7 +4228,7 @@
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"
@@ -5239,7 +5239,7 @@
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"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
