dang updated this revision to Diff 268746.
dang marked 3 inline comments as done.
dang added a comment.
Address some code review feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79796/new/
https://reviews.llvm.org/D79796
Files:
clang/include/clang/Driver/CC1Options.td
clang/include/clang/Frontend/CompilerInvocation.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/unittests/Frontend/CompilerInvocationTest.cpp
llvm/include/llvm/Option/OptParser.td
llvm/utils/TableGen/OptParserEmitter.cpp
Index: llvm/utils/TableGen/OptParserEmitter.cpp
===================================================================
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -9,6 +9,7 @@
#include "OptEmitter.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Twine.h"
#include "llvm/TableGen/Record.h"
#include "llvm/TableGen/TableGenBackend.h"
@@ -33,18 +34,18 @@
return OS;
}
-static void emitMarshallingInfo(raw_ostream &OS, const Record &R) {
- OS << R.getValueAsString("KeyPath");
+static void emitMarshallingInfoFlag(raw_ostream &OS, const Record *R) {
+ OS << R->getValueAsBit("IsPositive");
+ OS << ",";
+ OS << R->getValueAsString("DefaultValue");
+}
+
+static void emitMarshallingInfoString(raw_ostream &OS, const Record *R) {
+ OS << R->getValueAsString("DefaultValue");
OS << ", ";
- if (!isa<UnsetInit>(R.getValueInit("IsPositive")))
- OS << R.getValueAsBit("IsPositive");
- else
- OS << "INVALID";
+ OS << R->getValueAsString("Normalizer");
OS << ", ";
- if (!isa<UnsetInit>(R.getValueInit("DefaultValue")))
- OS << R.getValueAsString("DefaultValue");
- else
- OS << "INVALID";
+ OS << R->getValueAsString("Denormalizer");
}
/// OptParserEmitter - This tablegen backend takes an input .td file
@@ -246,15 +247,30 @@
OS << "#endif // OPTION\n";
OS << "#ifdef OPTION_WITH_MARSHALLING\n";
- for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
- const Record &R = *Opts[i];
+ for (unsigned I = 0, E = Opts.size(); I != E; ++I) {
+ const Record &R = *Opts[I];
if (!isa<UnsetInit>(R.getValueInit("MarshallingInfo"))) {
- OS << "OPTION_WITH_MARSHALLING(";
+ Record *MarshallingInfoRecord =
+ cast<DefInit>(R.getValueInit("MarshallingInfo"))->getDef();
+ StringRef KindStr = MarshallingInfoRecord->getValueAsString("Kind");
+ auto KindInfoPair =
+ StringSwitch<std::pair<
+ const char *, llvm::function_ref<void(raw_ostream &, Record *)>>>(
+ KindStr)
+ .Case("flag", std::make_pair("OPTION_WITH_MARSHALLING_FLAG",
+ &emitMarshallingInfoFlag))
+ .Case("string", std::make_pair("OPTION_WITH_MARSHALLING_STRING",
+ &emitMarshallingInfoString))
+ .Default(std::make_pair("", nullptr));
+ OS << KindInfoPair.first << "(";
WriteOptRecordFields(OS, R);
OS << ", ";
- emitMarshallingInfo(
- OS, *cast<DefInit>(R.getValueInit("MarshallingInfo"))->getDef());
+ OS << MarshallingInfoRecord->getValueAsBit("ShouldAlwaysEmit");
+ OS << ", ";
+ OS << MarshallingInfoRecord->getValueAsString("KeyPath");
+ OS << ", ";
+ KindInfoPair.second(OS, MarshallingInfoRecord);
OS << ")\n";
}
}
Index: llvm/include/llvm/Option/OptParser.td
===================================================================
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -81,13 +81,15 @@
}
// Add support for generating marshalling code
-
-class OptionMarshallingInfo<code keypath> {
+class OptionMarshallingInfo<string kind, code keypath> {
+ string Kind = kind;
+ bit ShouldAlwaysEmit = 0;
code KeyPath = keypath;
// Used by the Flag option kind.
bit IsPositive = ?;
code DefaultValue = ?;
- list<code> EnumValues = ?;
+ code Normalizer = ?;
+ code Denormalizer = ?;
}
// Define the option class.
@@ -143,20 +145,24 @@
class MarshallingInfo<OptionMarshallingInfo info> { OptionMarshallingInfo MarshallingInfo = info; }
class MarshallingFlag<code keypath, bit ispositive, code defaultvalue>
- : OptionMarshallingInfo<keypath> {
+ : OptionMarshallingInfo<"flag", keypath> {
bit IsPositive = ispositive;
code DefaultValue = defaultvalue;
}
-class MarshallingString<code keypath, code defaultvalue>
- : OptionMarshallingInfo<keypath> {
- code DefaultValue = defaultvalue;
+class MarshallingFlagAlwaysEmit<code keypath, bit ispositive, code defaultvalue>
+ : MarshallingFlag<keypath, ispositive, defaultvalue> {
+ let ShouldAlwaysEmit = 1;
}
-class MarshallingEnum<code keypath, code defaultvalue, list<code> enumvalues>
- : OptionMarshallingInfo<keypath> {
+class MarshallingString<code keypath, code defaultvalue, code normalizer, code denormalizer>
+ : OptionMarshallingInfo<"string", keypath> {
code DefaultValue = defaultvalue;
- list<code>EnumValues = enumvalues;
+ code Normalizer = normalizer;
+ code Denormalizer = denormalizer;
+}
+class MarshallingStringAlwaysEmit<code keypath, code defaultvalue, code normalizer, code denormalizer>
+ : MarshallingString<keypath, defaultvalue, normalizer, denormalizer> {
+ let ShouldAlwaysEmit = 1;
}
-
// Predefined options.
// FIXME: Have generator validate that these appear in correct position (and
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===================================================================
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -7,32 +7,96 @@
//===----------------------------------------------------------------------===//
#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Frontend/CompilerInstance.h"
+#include "llvm/Support/Host.h"
+#include "gmock/gmock.h"
#include "gtest/gtest.h"
using namespace llvm;
using namespace clang;
+using ::testing::Contains;
+using ::testing::Each;
+using ::testing::StrEq;
+using ::testing::StrNe;
+
namespace {
-TEST(CompilerInvocation, CanGenerateCC1CommandLine) {
+class CC1CommandLineGenerationTest : public ::testing::Test {
+public:
+ IntrusiveRefCntPtr<DiagnosticsEngine> Diags;
+ SmallVector<const char *, 32> GeneratedArgs;
+ SmallVector<std::string, 32> GeneratedArgsStorage;
+
+ const char *operator()(const Twine &Arg) {
+ return GeneratedArgsStorage.emplace_back(Arg.str()).c_str();
+ }
+
+ CC1CommandLineGenerationTest()
+ : Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions())) {}
+};
+
+TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineFlag) {
const char *Args[] = {"clang", "-xc++", "-fmodules-strict-context-hash", "-"};
- IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
- CompilerInstance::createDiagnostics(new DiagnosticOptions());
+ CompilerInvocation CInvok;
+ CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+ CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+ ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fmodules-strict-context-hash")));
+}
+
+TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineSeparate) {
+ const char *TripleCStr = "i686-apple-darwin9";
+ const char *Args[] = {"clang", "-xc++", "-triple", TripleCStr, "-"};
CompilerInvocation CInvok;
CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
- SmallVector<const char *, 32> GeneratedArgs;
- SmallVector<std::string, 32> GeneratedArgsStorage;
- auto StringAlloc = [&GeneratedArgsStorage](const Twine &Arg) {
- return GeneratedArgsStorage.emplace_back(Arg.str()).c_str();
- };
+ CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+ ASSERT_THAT(GeneratedArgs, Contains(StrEq(TripleCStr)));
+}
+
+TEST_F(CC1CommandLineGenerationTest,
+ CanGenerateCC1CommandLineSeparateRequired) {
+ const char *TripleCStr =
+ llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple()).c_str();
+ const char *Args[] = {"clang", "-xc++", "-triple", TripleCStr, "-"};
+
+ CompilerInvocation CInvok;
+ CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+ CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+ // Triple should always be emitted even if it is the default
+ ASSERT_THAT(GeneratedArgs, Contains(StrEq(TripleCStr)));
+}
+
+TEST_F(CC1CommandLineGenerationTest, CanGenerateCC1CommandLineSeparateEnum) {
+ const char *RelocationModelCStr = "static";
+ const char *Args[] = {"clang", "-xc++", "-mrelocation-model",
+ RelocationModelCStr, "-"};
+
+ CompilerInvocation CInvok;
+ CompilerInvocation::CreateFromArgs(CInvok, Args, *Diags);
+
+ CInvok.generateCC1CommandLine(GeneratedArgs, *this);
+
+ // Non default relocation model
+ ASSERT_THAT(GeneratedArgs, Contains(StrEq(RelocationModelCStr)));
+ GeneratedArgs.clear();
+
+ RelocationModelCStr = "pic";
+ Args[3] = RelocationModelCStr;
- CInvok.generateCC1CommandLine(GeneratedArgs, StringAlloc);
+ CompilerInvocation CInvok1;
+ CompilerInvocation::CreateFromArgs(CInvok1, Args, *Diags);
- ASSERT_STREQ(GeneratedArgs[0], "-fmodules-strict-context-hash");
+ CInvok1.generateCC1CommandLine(GeneratedArgs, *this);
+ ASSERT_THAT(GeneratedArgs, Each(StrNe(RelocationModelCStr)));
}
} // anonymous namespace
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -118,6 +118,55 @@
CompilerInvocationBase::~CompilerInvocationBase() = default;
+//===----------------------------------------------------------------------===//
+// Normalizers
+//===----------------------------------------------------------------------===//
+
+static std::string normalizeTriple(const Arg *Arg, const ArgList &ArgList,
+ DiagnosticsEngine &Diags,
+ StringRef DefaultTriple) {
+ return llvm::Triple::normalize(Arg->getValue());
+}
+
+static llvm::Reloc::Model
+normalizeRelocationModel(const Arg *Arg, const ArgList &ArgList,
+ DiagnosticsEngine &Diags,
+ llvm::Reloc::Model DefaultValue) {
+ StringRef ArgValue = Arg->getValue();
+ auto RM = llvm::StringSwitch<llvm::Optional<llvm::Reloc::Model>>(ArgValue)
+ .Case("static", llvm::Reloc::Static)
+ .Case("pic", llvm::Reloc::PIC_)
+ .Case("ropi", llvm::Reloc::ROPI)
+ .Case("rwpi", llvm::Reloc::RWPI)
+ .Case("ropi-rwpi", llvm::Reloc::ROPI_RWPI)
+ .Case("dynamic-no-pic", llvm::Reloc::DynamicNoPIC)
+ .Default(None);
+
+ if (RM.hasValue())
+ return *RM;
+
+ Diags.Report(diag::err_drv_invalid_value)
+ << Arg->getAsString(ArgList) << ArgValue;
+ return DefaultValue;
+}
+
+static const char *denormalizeRelocationModel(llvm::Reloc::Model RM) {
+ switch (RM) {
+ case llvm::Reloc::Static:
+ return "static";
+ case llvm::Reloc::PIC_:
+ return "pic";
+ case llvm::Reloc::DynamicNoPIC:
+ return "dynamic-no-pic";
+ case llvm::Reloc::ROPI:
+ return "ropi";
+ case llvm::Reloc::RWPI:
+ return "rwpi";
+ case llvm::Reloc::ROPI_RWPI:
+ return "ropi-rwpi";
+ }
+}
+
//===----------------------------------------------------------------------===//
// Deserialization (from args)
//===----------------------------------------------------------------------===//
@@ -529,25 +578,6 @@
Opts.ParseAllComments = Args.hasArg(OPT_fparse_all_comments);
}
-static llvm::Reloc::Model getRelocModel(ArgList &Args,
- DiagnosticsEngine &Diags) {
- if (Arg *A = Args.getLastArg(OPT_mrelocation_model)) {
- StringRef Value = A->getValue();
- auto RM = llvm::StringSwitch<llvm::Optional<llvm::Reloc::Model>>(Value)
- .Case("static", llvm::Reloc::Static)
- .Case("pic", llvm::Reloc::PIC_)
- .Case("ropi", llvm::Reloc::ROPI)
- .Case("rwpi", llvm::Reloc::RWPI)
- .Case("ropi-rwpi", llvm::Reloc::ROPI_RWPI)
- .Case("dynamic-no-pic", llvm::Reloc::DynamicNoPIC)
- .Default(None);
- if (RM.hasValue())
- return *RM;
- Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Value;
- }
- return llvm::Reloc::PIC_;
-}
-
/// Create a new Regex instance out of the string value in \p RpassArg.
/// It returns a pointer to the newly generated Regex instance.
static std::shared_ptr<llvm::Regex>
@@ -928,7 +958,6 @@
Opts.StrictVTablePointers = Args.hasArg(OPT_fstrict_vtable_pointers);
Opts.ForceEmitVTables = Args.hasArg(OPT_fforce_emit_vtables);
Opts.UnwindTables = Args.hasArg(OPT_munwind_tables);
- Opts.RelocationModel = getRelocModel(Args, Diags);
Opts.ThreadModel =
std::string(Args.getLastArgValue(OPT_mthread_model, "posix"));
if (Opts.ThreadModel != "posix" && Opts.ThreadModel != "single")
@@ -3613,6 +3642,32 @@
}
}
+bool CompilerInvocation::parseSimpleArgs(const ArgList &Args,
+ DiagnosticsEngine &Diags) {
+#define OPTION_WITH_MARSHALLING
+#define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP, \
+ ALIAS, ALIASARGS, FLAGS, PARAM, HELPTEXT, \
+ METAVAR, VALUES, ALWAYS_EMIT, KEYPATH, \
+ IS_POSITIVE, DEFAULT_VALUE) \
+ this->KEYPATH = Args.hasArg(OPT_##ID) && IS_POSITIVE;
+
+#define OPTION_WITH_MARSHALLING_STRING( \
+ PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
+ HELPTEXT, METAVAR, VALUES, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, \
+ NORMALIZER, DENORMALIZER) \
+ { \
+ auto Arg = Args.getLastArg(OPT_##ID); \
+ this->KEYPATH = \
+ !Arg ? DEFAULT_VALUE : NORMALIZER(Arg, Args, Diags, DEFAULT_VALUE); \
+ }
+
+#include "clang/Driver/Options.inc"
+#undef OPTION_WITH_MARSHALLING_STRING
+#undef OPTION_WITH_MARSHALLING_FLAG
+#undef OPTION_WITH_MARSHALLING
+ return true;
+}
+
bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
ArrayRef<const char *> CommandLineArgs,
DiagnosticsEngine &Diags) {
@@ -3624,15 +3679,6 @@
unsigned MissingArgIndex, MissingArgCount;
InputArgList Args = Opts.ParseArgs(CommandLineArgs, MissingArgIndex,
MissingArgCount, IncludedFlagsBitmask);
-
-#define OPTION_WITH_MARSHALLING(PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, \
- ALIASARGS, FLAGS, PARAM, HELPTEXT, METAVAR, \
- VALUES, KEYPATH, IS_POSITIVE, DEFAULT_VALUE) \
- if (Option::KIND##Class == Option::FlagClass) \
- Res.KEYPATH = Args.hasArg(OPT_##ID) && IS_POSITIVE;
-#include "clang/Driver/Options.inc"
-#undef OPTION_WITH_MARSHALLING
-
LangOptions &LangOpts = *Res.getLangOpts();
// Check for missing argument error.
@@ -3654,6 +3700,7 @@
Success = false;
}
+ Success &= Res.parseSimpleArgs(Args, Diags);
Success &= ParseAnalyzerArgs(*Res.getAnalyzerOpts(), Args, Diags);
Success &= ParseMigratorArgs(Res.getMigratorOpts(), Args);
ParseDependencyOutputArgs(Res.getDependencyOutputOpts(), Args);
@@ -3861,13 +3908,31 @@
llvm::function_ref<const char *(const Twine &)> StringAllocator) const {
#define PREFIX(PREFIX_TYPE, BRACED_INIT) \
const char *PREFIX_TYPE[4] = BRACED_INIT;
-#define OPTION_WITH_MARSHALLING(PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, \
- ALIASARGS, FLAGS, PARAM, HELPTEXT, METAVAR, \
- VALUES, KEYPATH, IS_POSITIVE, DEFAULT_VALUE) \
- if (Option::KIND##Class == Option::FlagClass && \
- IS_POSITIVE != DEFAULT_VALUE && this->KEYPATH != DEFAULT_VALUE) \
+
+#define OPTION_WITH_MARSHALLING
+#define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP, \
+ ALIAS, ALIASARGS, FLAGS, PARAM, HELPTEXT, \
+ METAVAR, VALUES, ALWAYS_EMIT, KEYPATH, \
+ IS_POSITIVE, DEFAULT_VALUE) \
+ if (FLAGS & options::CC1Option && IS_POSITIVE != DEFAULT_VALUE && \
+ (this->KEYPATH != DEFAULT_VALUE || ALWAYS_EMIT)) \
Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME));
+
+#define OPTION_WITH_MARSHALLING_STRING( \
+ PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
+ HELPTEXT, METAVAR, VALUES, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, \
+ NORMALIZER, DENORMALIZER) \
+ if ((FLAGS & options::CC1Option) && \
+ (this->KEYPATH != DEFAULT_VALUE || ALWAYS_EMIT)) { \
+ if (Option::KIND##Class == Option::SeparateClass) { \
+ Args.push_back(StringAllocator(Twine(PREFIX_TYPE[0]) + NAME)); \
+ Args.push_back(StringAllocator(DENORMALIZER(this->KEYPATH))); \
+ } \
+ }
+
#include "clang/Driver/Options.inc"
+#undef OPTION_WITH_MARSHALLING_STRING
+#undef OPTION_WITH_MARSHALLING_FLAG
#undef OPTION_WITH_MARSHALLING
#undef PREFIX
}
Index: clang/include/clang/Frontend/CompilerInvocation.h
===================================================================
--- clang/include/clang/Frontend/CompilerInvocation.h
+++ clang/include/clang/Frontend/CompilerInvocation.h
@@ -237,6 +237,16 @@
}
/// @}
+
+private:
+ /// Parse options for flags that expose marshalling information in their
+ /// table-gen definition
+ ///
+ /// \param Args - The argument list containing the arguments to parse
+ /// \param Diags - The DiagnosticsEngine associated with CreateFromArgs
+ /// \returns - True if parsing was successful, false otherwise
+ bool parseSimpleArgs(const llvm::opt::ArgList &Args,
+ DiagnosticsEngine &Diags);
};
IntrusiveRefCntPtr<llvm::vfs::FileSystem>
Index: clang/include/clang/Driver/CC1Options.td
===================================================================
--- clang/include/clang/Driver/CC1Options.td
+++ clang/include/clang/Driver/CC1Options.td
@@ -23,7 +23,8 @@
def target_feature : Separate<["-"], "target-feature">,
HelpText<"Target specific attributes">;
def triple : Separate<["-"], "triple">,
- HelpText<"Specify target triple (e.g. i686-apple-darwin9)">;
+ HelpText<"Specify target triple (e.g. i686-apple-darwin9)">,
+ MarshallingInfo<MarshallingStringAlwaysEmit<"TargetOpts->Triple", "llvm::sys::getDefaultTargetTriple()", "normalizeTriple", "">>;
def target_abi : Separate<["-"], "target-abi">,
HelpText<"Target a particular ABI type">;
def target_sdk_version_EQ : Joined<["-"], "target-sdk-version=">,
@@ -212,7 +213,8 @@
"Note this may change .s semantics and shouldn't generally be used "
"on compiler-generated code.">;
def mrelocation_model : Separate<["-"], "mrelocation-model">,
- HelpText<"The relocation model to use">, Values<"static,pic,ropi,rwpi,ropi-rwpi,dynamic-no-pic">;
+ HelpText<"The relocation model to use">, Values<"static,pic,ropi,rwpi,ropi-rwpi,dynamic-no-pic">,
+ MarshallingInfo<MarshallingString<"CodeGenOpts.RelocationModel", "llvm::Reloc::PIC_", "normalizeRelocationModel", "denormalizeRelocationModel">>;
def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
HelpText<"Disable implicit builtin knowledge of math functions">;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
