[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
thakis added a comment. Herald added a project: clang. I noticed that this causes memory errors in certain situations. https://bugs.llvm.org/show_bug.cgi?id=42501 has details. Can you take a look? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53457/new/ https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346393: clang-cl: Add /clang: pass-through arg
support. (authored by hans, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D53457
Files:
docs/UsersManual.rst
include/clang/Driver/CLCompatOptions.td
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
test/Driver/cl-options.c
Index: include/clang/Driver/CLCompatOptions.td
===
--- include/clang/Driver/CLCompatOptions.td
+++ include/clang/Driver/CLCompatOptions.td
@@ -321,6 +321,8 @@
def _SLASH_volatile_ms : Option<["/", "-"], "volatile:ms", KIND_FLAG>,
Group<_SLASH_volatile_Group>, Flags<[CLOption, DriverOption]>,
HelpText<"Volatile loads and stores have acquire and release semantics">;
+def _SLASH_clang : CLJoined<"clang:">,
+ HelpText<"Pass to the clang driver">, MetaVarName<"">;
def _SLASH_Zl : CLFlag<"Zl">,
HelpText<"Don't mention any default libraries in the object file">;
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -362,6 +362,7 @@
/// ParseArgStrings - Parse the given list of strings into an
/// ArgList.
llvm::opt::InputArgList ParseArgStrings(ArrayRef Args,
+ bool IsClCompatMode,
bool );
/// BuildInputs - Construct the list of inputs and their types from
@@ -552,7 +553,7 @@
/// Get bitmasks for which option flags to include and exclude based on
/// the driver mode.
- std::pair getIncludeExcludeOptionFlagMasks() const;
+ std::pair getIncludeExcludeOptionFlagMasks(bool IsClCompatMode) const;
/// Helper used in BuildJobsForAction. Doesn't use the cache when building
/// jobs specifically for the given action, but will use the cache when
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -619,5 +619,19 @@
// RUN: --version \
// RUN: -Werror /Zs -- %s 2>&1
+// Accept clang options under the /clang: flag.
+// The first test case ensures that the SLP vectorizer is on by default and that
+// it's being turned off by the /clang:-fno-slp-vectorize flag.
+
+// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s
+// NOCLANG: "--dependent-lib=libcmt"
+// NOCLANG-SAME: "-vectorize-slp"
+// NOCLANG-NOT: "--dependent-lib=msvcrt"
+
+// RUN: %clang_cl -O2 -MD /clang:-fno-slp-vectorize /clang:-MD /clang:-MF /clang:my_dependency_file.dep -### -- %s 2>&1 | FileCheck -check-prefix=CLANG %s
+// CLANG: "--dependent-lib=msvcrt"
+// CLANG-SAME: "-dependency-file" "my_dependency_file.dep"
+// CLANG-NOT: "--dependent-lib=libcmt"
+// CLANG-NOT: "-vectorize-slp"
void f() { }
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -166,14 +166,15 @@
}
InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings,
+ bool IsClCompatMode,
bool ) {
llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
ContainsError = false;
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsClCompatMode);
unsigned MissingArgIndex, MissingArgCount;
InputArgList Args =
@@ -730,7 +731,7 @@
ConfigFile = CfgFileName.str();
bool ContainErrors;
CfgOptions = llvm::make_unique(
- ParseArgStrings(NewCfgArgs, ContainErrors));
+ ParseArgStrings(NewCfgArgs, IsCLMode(), ContainErrors));
if (ContainErrors) {
CfgOptions.reset();
return true;
@@ -924,7 +925,7 @@
// Arguments specified in command line.
bool ContainsError;
CLOptions = llvm::make_unique(
- ParseArgStrings(ArgList.slice(1), ContainsError));
+ ParseArgStrings(ArgList.slice(1), IsCLMode(), ContainsError));
// Try parsing configuration file.
if (!ContainsError)
@@ -934,21 +935,47 @@
// All arguments, from both config file and command line.
InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
: std::move(*CLOptions));
- if (HasConfigFile)
-for (auto *Opt : *CLOptions) {
- if (Opt->getOption().matches(options::OPT_config))
-continue;
+
+ auto appendOneArg = [](const Arg *Opt, const Arg *BaseArg) {
unsigned Index = Args.MakeIndex(Opt->getSpelling());
- const Arg *BaseArg = >getBaseArg();
- if (BaseArg == Opt)
-BaseArg = nullptr;
Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Opt->getSpelling(),
Index,
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
hans added a comment. In https://reviews.llvm.org/D53457#1291020, @neerajksingh wrote: > Reid, Hans, or someone else with commit access. If the revision looks good, > could you please submit to SVN? > > Any particular testing I should run beforehand? I ran the clang tests locally > on Windows. I'll run the tests on Linux and commit. https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
neerajksingh added a comment. Reid, Hans, or someone else with commit access. If the revision looks good, could you please submit to SVN? Any particular testing I should run beforehand? I ran the clang tests locally on Windows. https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
neerajksingh updated this revision to Diff 173072.
neerajksingh marked an inline comment as done.
https://reviews.llvm.org/D53457
Files:
docs/UsersManual.rst
include/clang/Driver/CLCompatOptions.td
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
test/Driver/cl-options.c
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -614,5 +614,19 @@
// RUN: --version \
// RUN: -Werror /Zs -- %s 2>&1
+// Accept clang options under the /clang: flag.
+// The first test case ensures that the SLP vectorizer is on by default and that
+// it's being turned off by the /clang:-fno-slp-vectorize flag.
+
+// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s
+// NOCLANG: "--dependent-lib=libcmt"
+// NOCLANG-SAME: "-vectorize-slp"
+// NOCLANG-NOT: "--dependent-lib=msvcrt"
+
+// RUN: %clang_cl -O2 -MD /clang:-fno-slp-vectorize /clang:-MD /clang:-MF /clang:my_dependency_file.dep -### -- %s 2>&1 | FileCheck -check-prefix=CLANG %s
+// CLANG: "--dependent-lib=msvcrt"
+// CLANG-SAME: "-dependency-file" "my_dependency_file.dep"
+// CLANG-NOT: "--dependent-lib=libcmt"
+// CLANG-NOT: "-vectorize-slp"
void f() { }
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -166,14 +166,15 @@
}
InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings,
+ bool IsClCompatMode,
bool ) {
llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
ContainsError = false;
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsClCompatMode);
unsigned MissingArgIndex, MissingArgCount;
InputArgList Args =
@@ -730,7 +731,7 @@
ConfigFile = CfgFileName.str();
bool ContainErrors;
CfgOptions = llvm::make_unique(
- ParseArgStrings(NewCfgArgs, ContainErrors));
+ ParseArgStrings(NewCfgArgs, IsCLMode(), ContainErrors));
if (ContainErrors) {
CfgOptions.reset();
return true;
@@ -924,7 +925,7 @@
// Arguments specified in command line.
bool ContainsError;
CLOptions = llvm::make_unique(
- ParseArgStrings(ArgList.slice(1), ContainsError));
+ ParseArgStrings(ArgList.slice(1), IsCLMode(), ContainsError));
// Try parsing configuration file.
if (!ContainsError)
@@ -934,21 +935,47 @@
// All arguments, from both config file and command line.
InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
: std::move(*CLOptions));
- if (HasConfigFile)
-for (auto *Opt : *CLOptions) {
- if (Opt->getOption().matches(options::OPT_config))
-continue;
+
+ auto appendOneArg = [](const Arg *Opt, const Arg *BaseArg) {
unsigned Index = Args.MakeIndex(Opt->getSpelling());
- const Arg *BaseArg = >getBaseArg();
- if (BaseArg == Opt)
-BaseArg = nullptr;
Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Opt->getSpelling(),
Index, BaseArg);
Copy->getValues() = Opt->getValues();
if (Opt->isClaimed())
Copy->claim();
Args.append(Copy);
+ };
+
+ if (HasConfigFile)
+for (auto *Opt : *CLOptions) {
+ if (Opt->getOption().matches(options::OPT_config))
+continue;
+ const Arg *BaseArg = >getBaseArg();
+ if (BaseArg == Opt)
+BaseArg = nullptr;
+ appendOneArg(Opt, BaseArg);
+}
+
+ // In CL mode, look for any pass-through arguments
+ if (IsCLMode() && !ContainsError) {
+SmallVector CLModePassThroughArgList;
+for (const auto *A : Args.filtered(options::OPT__SLASH_clang)) {
+ A->claim();
+ CLModePassThroughArgList.push_back(A->getValue());
+}
+
+if (!CLModePassThroughArgList.empty()) {
+ // Parse any pass through args using default clang processing rather
+ // than clang-cl processing.
+ auto CLModePassThroughOptions = llvm::make_unique(
+ ParseArgStrings(CLModePassThroughArgList, false, ContainsError));
+
+ if (!ContainsError)
+for (auto *Opt : *CLModePassThroughOptions) {
+ appendOneArg(Opt, nullptr);
+}
}
+ }
// FIXME: This stuff needs to go into the Compilation, not the driver.
bool CCCPrintPhases;
@@ -1452,7 +1479,7 @@
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsCLMode());
ExcludedFlagsBitmask |= options::NoDriverOption;
if (!ShowHidden)
@@ -4661,11 +4688,11 @@
return false;
}
-std::pair Driver::getIncludeExcludeOptionFlagMasks() const {
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
neerajksingh marked 2 inline comments as done. neerajksingh added a comment. Will update revision... Comment at: test/Driver/cl-options.c:619 + +// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s +// NOCLANG: "--dependent-lib=libcmt" hans wrote: > I'm not sure this test adds much value.. This test ensures that the next test is actually testing something (i.e. that the settings change between the two runs). https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Okay, looks good to me (with a small nit). Comment at: docs/UsersManual.rst:2852 /Brepro Emit an object file which can be reproduced over time + /clang:Pass to the clang driver /C Don't discard comments when preprocessing ultra nit: no space between the colon and Comment at: test/Driver/cl-options.c:619 + +// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s +// NOCLANG: "--dependent-lib=libcmt" I'm not sure this test adds much value.. https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
neerajksingh updated this revision to Diff 172615.
neerajksingh added a comment.
Make it clear in the documentation that the /clang flags are added to the end.
https://reviews.llvm.org/D53457
Files:
docs/UsersManual.rst
include/clang/Driver/CLCompatOptions.td
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
test/Driver/cl-options.c
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -614,5 +614,17 @@
// RUN: --version \
// RUN: -Werror /Zs -- %s 2>&1
+// Accept clang options under the /clang: flag.
+
+// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s
+// NOCLANG: "--dependent-lib=libcmt"
+// NOCLANG-SAME: "-vectorize-slp"
+// NOCLANG-NOT: "--dependent-lib=msvcrt"
+
+// RUN: %clang_cl -O2 -MD /clang:-fno-slp-vectorize /clang:-MD /clang:-MF /clang:my_dependency_file.dep -### -- %s 2>&1 | FileCheck -check-prefix=CLANG %s
+// CLANG: "--dependent-lib=msvcrt"
+// CLANG-SAME: "-dependency-file" "my_dependency_file.dep"
+// CLANG-NOT: "--dependent-lib=libcmt"
+// CLANG-NOT: "-vectorize-slp"
void f() { }
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -166,14 +166,15 @@
}
InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings,
+ bool IsClCompatMode,
bool ) {
llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
ContainsError = false;
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsClCompatMode);
unsigned MissingArgIndex, MissingArgCount;
InputArgList Args =
@@ -730,7 +731,7 @@
ConfigFile = CfgFileName.str();
bool ContainErrors;
CfgOptions = llvm::make_unique(
- ParseArgStrings(NewCfgArgs, ContainErrors));
+ ParseArgStrings(NewCfgArgs, IsCLMode(), ContainErrors));
if (ContainErrors) {
CfgOptions.reset();
return true;
@@ -924,7 +925,7 @@
// Arguments specified in command line.
bool ContainsError;
CLOptions = llvm::make_unique(
- ParseArgStrings(ArgList.slice(1), ContainsError));
+ ParseArgStrings(ArgList.slice(1), IsCLMode(), ContainsError));
// Try parsing configuration file.
if (!ContainsError)
@@ -934,21 +935,47 @@
// All arguments, from both config file and command line.
InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
: std::move(*CLOptions));
- if (HasConfigFile)
-for (auto *Opt : *CLOptions) {
- if (Opt->getOption().matches(options::OPT_config))
-continue;
+
+ auto appendOneArg = [](const Arg *Opt, const Arg *BaseArg) {
unsigned Index = Args.MakeIndex(Opt->getSpelling());
- const Arg *BaseArg = >getBaseArg();
- if (BaseArg == Opt)
-BaseArg = nullptr;
Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Opt->getSpelling(),
Index, BaseArg);
Copy->getValues() = Opt->getValues();
if (Opt->isClaimed())
Copy->claim();
Args.append(Copy);
+ };
+
+ if (HasConfigFile)
+for (auto *Opt : *CLOptions) {
+ if (Opt->getOption().matches(options::OPT_config))
+continue;
+ const Arg *BaseArg = >getBaseArg();
+ if (BaseArg == Opt)
+BaseArg = nullptr;
+ appendOneArg(Opt, BaseArg);
+}
+
+ // In CL mode, look for any pass-through arguments
+ if (IsCLMode() && !ContainsError) {
+SmallVector CLModePassThroughArgList;
+for (const auto *A : Args.filtered(options::OPT__SLASH_clang)) {
+ A->claim();
+ CLModePassThroughArgList.push_back(A->getValue());
+}
+
+if (!CLModePassThroughArgList.empty()) {
+ // Parse any pass through args using default clang processing rather
+ // than clang-cl processing.
+ auto CLModePassThroughOptions = llvm::make_unique(
+ ParseArgStrings(CLModePassThroughArgList, false, ContainsError));
+
+ if (!ContainsError)
+for (auto *Opt : *CLModePassThroughOptions) {
+ appendOneArg(Opt, nullptr);
+}
}
+ }
// FIXME: This stuff needs to go into the Compilation, not the driver.
bool CCCPrintPhases;
@@ -1452,7 +1479,7 @@
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsCLMode());
ExcludedFlagsBitmask |= options::NoDriverOption;
if (!ShowHidden)
@@ -4661,11 +4688,11 @@
return false;
}
-std::pair Driver::getIncludeExcludeOptionFlagMasks() const {
+std::pair Driver::getIncludeExcludeOptionFlagMasks(bool IsClCompatMode) const {
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
hans added a comment. In https://reviews.llvm.org/D53457#1279191, @neerajksingh wrote: > In https://reviews.llvm.org/D53457#1278579, @hans wrote: > > > The `-Xclang` option has the same issue, and I think `/clang:` should work > > similarly, i.e. `/clang:-MF /clang:`. It's not pretty, but at > > least it's consistent. Yes, that means processing consecutive `/Xclang:` > > options together, but hopefully that's doable. > > > It looks like the handling for -Xclang is a lot simpler (in > `Clang::ConstructJob`). There the Xclang options are all gathered and > forwarded at a particular spot in the command line for cc1. They aren't > interleaved with other options, and it wouldn't really make sense to do so > anyway since it doesn't really look like cc1 arguments are constructed from > driver arguments in any particular order. > > The llvm/lib/Option/OptTable.cpp code is not really setup to easily > interleave arguments like would be required for your request. Can you see a > way to accomplish what you want without a deep refactoring of > OptTable::ParseArgs and the InputArgList class? Okay, if it's hard to do, I suppose collecting the /clang: options and processing them separately after the others is the second-best option. We'll need to make sure the behaviour is well-documented. https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
neerajksingh added a comment. In https://reviews.llvm.org/D53457#1278579, @hans wrote: > The `-Xclang` option has the same issue, and I think `/clang:` should work > similarly, i.e. `/clang:-MF /clang:`. It's not pretty, but at least > it's consistent. Yes, that means processing consecutive `/Xclang:` options > together, but hopefully that's doable. It looks like the handling for -Xclang is a lot simpler (in `Clang::ConstructJob`). There the Xclang options are all gathered and forwarded at a particular spot in the command line for cc1. They aren't interleaved with other options, and it wouldn't really make sense to do so anyway since it doesn't really look like cc1 arguments are constructed from driver arguments in any particular order. The llvm/lib/Option/OptTable.cpp code is not really setup to easily interleave arguments like would be required for your request. Can you see a way to accomplish what you want without a deep refactoring of OptTable::ParseArgs and the InputArgList class? https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
hans added a comment. In https://reviews.llvm.org/D53457#1277950, @neerajksingh wrote: > In https://reviews.llvm.org/D53457#1277315, @hans wrote: > > > One note about flag ordering: the /clang: flags are concatenated to the > > end of > > the argument list, so in cases where the last flag wins, the /clang: flags > > will be chosen regardless of their order relative to other flags on the > > driver > > command line. > > > > > > This seems a little unfortunate. I wonder if it would be possible to not > > have this restriction, i.e. to process these in-line with the rest of the > > flags? > > > > One way to achieve this would be to change Driver::ParseArgStrings() to > > handle the "/clang:" arguments. After doing the regular option parsing, it > > would look for "/clang:" options and substitute them for the underlying > > option. This has the downside of adding some option-specific logic to > > ParseArgStrings, but on the other hand it's less intrusive than passing > > around the IsCLMode bool. Do you think something like this could work? > > > One difficulty occurs with options that have separated values, like the > `/clang:-MF /clang:` case. In this case, we need to take two > /clang: arguments and process them next to each other in order to form a > single coherent argument/value pair. Another option is to allow the user to > specify the option like `/clang:"-MF "` and then require that the > user specify any flags that need a value in a single /clang: argument. This > would require some code to split the value string on spaces before passing it > on to clang argument parsing. > > Do you have any preference or better suggestion for the option-with-value > case? The `-Xclang` option has the same issue, and I think `/clang:` should work similarly, i.e. `/clang:-MF /clang:`. It's not pretty, but at least it's consistent. Yes, that means processing consecutive `/Xclang:` options together, but hopefully that's doable. https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
neerajksingh updated this revision to Diff 171382.
neerajksingh added a comment.
Fix hans' comments.
https://reviews.llvm.org/D53457
Files:
docs/UsersManual.rst
include/clang/Driver/CLCompatOptions.td
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
test/Driver/cl-options.c
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -614,5 +614,17 @@
// RUN: --version \
// RUN: -Werror /Zs -- %s 2>&1
+// Accept clang options under the /clang: flag.
+
+// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s
+// NOCLANG: "--dependent-lib=libcmt"
+// NOCLANG-SAME: "-vectorize-slp"
+// NOCLANG-NOT: "--dependent-lib=msvcrt"
+
+// RUN: %clang_cl -O2 -MD /clang:-fno-slp-vectorize /clang:-MD /clang:-MF /clang:my_dependency_file.dep -### -- %s 2>&1 | FileCheck -check-prefix=CLANG %s
+// CLANG: "--dependent-lib=msvcrt"
+// CLANG-SAME: "-dependency-file" "my_dependency_file.dep"
+// CLANG-NOT: "--dependent-lib=libcmt"
+// CLANG-NOT: "-vectorize-slp"
void f() { }
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -166,14 +166,15 @@
}
InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings,
+ bool IsClCompatMode,
bool ) {
llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
ContainsError = false;
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsClCompatMode);
unsigned MissingArgIndex, MissingArgCount;
InputArgList Args =
@@ -730,7 +731,7 @@
ConfigFile = CfgFileName.str();
bool ContainErrors;
CfgOptions = llvm::make_unique(
- ParseArgStrings(NewCfgArgs, ContainErrors));
+ ParseArgStrings(NewCfgArgs, IsCLMode(), ContainErrors));
if (ContainErrors) {
CfgOptions.reset();
return true;
@@ -924,7 +925,7 @@
// Arguments specified in command line.
bool ContainsError;
CLOptions = llvm::make_unique(
- ParseArgStrings(ArgList.slice(1), ContainsError));
+ ParseArgStrings(ArgList.slice(1), IsCLMode(), ContainsError));
// Try parsing configuration file.
if (!ContainsError)
@@ -934,21 +935,47 @@
// All arguments, from both config file and command line.
InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
: std::move(*CLOptions));
- if (HasConfigFile)
-for (auto *Opt : *CLOptions) {
- if (Opt->getOption().matches(options::OPT_config))
-continue;
+
+ auto appendOneArg = [](const Arg *Opt, const Arg *BaseArg) {
unsigned Index = Args.MakeIndex(Opt->getSpelling());
- const Arg *BaseArg = >getBaseArg();
- if (BaseArg == Opt)
-BaseArg = nullptr;
Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Opt->getSpelling(),
Index, BaseArg);
Copy->getValues() = Opt->getValues();
if (Opt->isClaimed())
Copy->claim();
Args.append(Copy);
+ };
+
+ if (HasConfigFile)
+for (auto *Opt : *CLOptions) {
+ if (Opt->getOption().matches(options::OPT_config))
+continue;
+ const Arg *BaseArg = >getBaseArg();
+ if (BaseArg == Opt)
+BaseArg = nullptr;
+ appendOneArg(Opt, BaseArg);
+}
+
+ // In CL mode, look for any pass-through arguments
+ if (IsCLMode() && !ContainsError) {
+SmallVector CLModePassThroughArgList;
+for (const auto *A : Args.filtered(options::OPT__SLASH_clang)) {
+ A->claim();
+ CLModePassThroughArgList.push_back(A->getValue());
+}
+
+if (!CLModePassThroughArgList.empty()) {
+ // Parse any pass through args using default clang processing rather
+ // than clang-cl processing.
+ auto CLModePassThroughOptions = llvm::make_unique(
+ ParseArgStrings(CLModePassThroughArgList, false, ContainsError));
+
+ if (!ContainsError)
+for (auto *Opt : *CLModePassThroughOptions) {
+ appendOneArg(Opt, nullptr);
+}
}
+ }
// FIXME: This stuff needs to go into the Compilation, not the driver.
bool CCCPrintPhases;
@@ -1452,7 +1479,7 @@
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsCLMode());
ExcludedFlagsBitmask |= options::NoDriverOption;
if (!ShowHidden)
@@ -4661,11 +4688,11 @@
return false;
}
-std::pair Driver::getIncludeExcludeOptionFlagMasks() const {
+std::pair Driver::getIncludeExcludeOptionFlagMasks(bool IsClCompatMode) const {
unsigned IncludedFlagsBitmask = 0;
unsigned
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
neerajksingh added a comment. In https://reviews.llvm.org/D53457#1277315, @hans wrote: > One note about flag ordering: the /clang: flags are concatenated to the end > of > the argument list, so in cases where the last flag wins, the /clang: flags > will be chosen regardless of their order relative to other flags on the > driver > command line. > > > This seems a little unfortunate. I wonder if it would be possible to not have > this restriction, i.e. to process these in-line with the rest of the flags? > > One way to achieve this would be to change Driver::ParseArgStrings() to > handle the "/clang:" arguments. After doing the regular option parsing, it > would look for "/clang:" options and substitute them for the underlying > option. This has the downside of adding some option-specific logic to > ParseArgStrings, but on the other hand it's less intrusive than passing > around the IsCLMode bool. Do you think something like this could work? One difficulty occurs with options that have separated values, like the `/clang:-MF /clang:` case. In this case, we need to take two /clang: arguments and process them next to each other in order to form a single coherent argument/value pair. Another option is to allow the user to specify the option like `/clang:"-MF "` and then require that the user specify any flags that need a value in a single /clang: argument. This would require some code to split the value string on spaces before passing it on to clang argument parsing. Do you have any preference or better suggestion for the option-with-value case? https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
hans added a comment. One note about flag ordering: the /clang: flags are concatenated to the end of the argument list, so in cases where the last flag wins, the /clang: flags will be chosen regardless of their order relative to other flags on the driver command line. This seems a little unfortunate. I wonder if it would be possible to not have this restriction, i.e. to process these in-line with the rest of the flags? One way to achieve this would be to change Driver::ParseArgStrings() to handle the "/clang:" arguments. After doing the regular option parsing, it would look for "/clang:" options and substitute them for the underlying option. This has the downside of adding some option-specific logic to ParseArgStrings, but on the other hand it's less intrusive than passing around the IsCLMode bool. Do you think something like this could work? Comment at: docs/UsersManual.rst:3100 + +The /clang: Option + Thanks for thinking about the docs! I think we should put this above the /fallback section, since this new flag is more important and /fallback shouldn't be used much these days. Comment at: include/clang/Driver/CLCompatOptions.td:324 HelpText<"Volatile loads and stores have acquire and release semantics">; +def _SLASH_clang : CLJoinedOrSeparate<"clang:">, + HelpText<"Pass to the clang driver">, MetaVarName<"">; Do we really want the "OrSeparate" part of this? Is there any downside of limiting it to "/clang:-foo" rather than also allowing "/clang: -foo"? https://reviews.llvm.org/D53457 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
neerajksingh updated this revision to Diff 171233.
https://reviews.llvm.org/D53457
Files:
docs/UsersManual.rst
include/clang/Driver/CLCompatOptions.td
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
test/Driver/cl-options.c
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -614,5 +614,17 @@
// RUN: --version \
// RUN: -Werror /Zs -- %s 2>&1
+// Accept clang options under the /clang: flag.
+
+// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s
+// NOCLANG: "--dependent-lib=libcmt"
+// NOCLANG-SAME: "-vectorize-slp"
+// NOCLANG-NOT: "--dependent-lib=msvcrt"
+
+// RUN: %clang_cl -O2 -MD /clang:-fno-slp-vectorize /clang:-MD /clang:-MF /clang:my_dependency_file.dep -### -- %s 2>&1 | FileCheck -check-prefix=CLANG %s
+// CLANG: "--dependent-lib=msvcrt"
+// CLANG-SAME: "-dependency-file" "my_dependency_file.dep"
+// CLANG-NOT: "--dependent-lib=libcmt"
+// CLANG-NOT: "-vectorize-slp"
void f() { }
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -166,14 +166,15 @@
}
InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings,
+ bool IsClCompatMode,
bool ) {
llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
ContainsError = false;
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsClCompatMode);
unsigned MissingArgIndex, MissingArgCount;
InputArgList Args =
@@ -730,7 +731,7 @@
ConfigFile = CfgFileName.str();
bool ContainErrors;
CfgOptions = llvm::make_unique(
- ParseArgStrings(NewCfgArgs, ContainErrors));
+ ParseArgStrings(NewCfgArgs, IsCLMode(), ContainErrors));
if (ContainErrors) {
CfgOptions.reset();
return true;
@@ -924,7 +925,7 @@
// Arguments specified in command line.
bool ContainsError;
CLOptions = llvm::make_unique(
- ParseArgStrings(ArgList.slice(1), ContainsError));
+ ParseArgStrings(ArgList.slice(1), IsCLMode(), ContainsError));
// Try parsing configuration file.
if (!ContainsError)
@@ -934,21 +935,47 @@
// All arguments, from both config file and command line.
InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
: std::move(*CLOptions));
- if (HasConfigFile)
-for (auto *Opt : *CLOptions) {
- if (Opt->getOption().matches(options::OPT_config))
-continue;
+
+ auto appendOneArg = [](const Arg *Opt, const Arg *BaseArg) {
unsigned Index = Args.MakeIndex(Opt->getSpelling());
- const Arg *BaseArg = >getBaseArg();
- if (BaseArg == Opt)
-BaseArg = nullptr;
Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Opt->getSpelling(),
Index, BaseArg);
Copy->getValues() = Opt->getValues();
if (Opt->isClaimed())
Copy->claim();
Args.append(Copy);
+ };
+
+ if (HasConfigFile)
+for (auto *Opt : *CLOptions) {
+ if (Opt->getOption().matches(options::OPT_config))
+continue;
+ const Arg *BaseArg = >getBaseArg();
+ if (BaseArg == Opt)
+BaseArg = nullptr;
+ appendOneArg(Opt, BaseArg);
+}
+
+ // In CL mode, look for any pass-through arguments
+ if (IsCLMode() && !ContainsError) {
+SmallVector CLModePassThroughArgList;
+for (const auto *A : Args.filtered(options::OPT__SLASH_clang)) {
+ A->claim();
+ CLModePassThroughArgList.push_back(A->getValue());
+}
+
+if (!CLModePassThroughArgList.empty()) {
+ // Parse any pass through args using default clang processing rather
+ // than clang-cl processing.
+ auto CLModePassThroughOptions = llvm::make_unique(
+ ParseArgStrings(CLModePassThroughArgList, false, ContainsError));
+
+ if (!ContainsError)
+for (auto *Opt : *CLModePassThroughOptions) {
+ appendOneArg(Opt, nullptr);
+}
}
+ }
// FIXME: This stuff needs to go into the Compilation, not the driver.
bool CCCPrintPhases;
@@ -1452,7 +1479,7 @@
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsCLMode());
ExcludedFlagsBitmask |= options::NoDriverOption;
if (!ShowHidden)
@@ -4661,11 +4688,11 @@
return false;
}
-std::pair Driver::getIncludeExcludeOptionFlagMasks() const {
+std::pair Driver::getIncludeExcludeOptionFlagMasks(bool IsClCompatMode) const {
unsigned IncludedFlagsBitmask = 0;
unsigned ExcludedFlagsBitmask = options::NoDriverOption;
- if (Mode ==
[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.
neerajksingh updated this revision to Diff 171203.
neerajksingh retitled this revision from "clang-cl: Add "/Xdriver:"
pass-through arg support." to "clang-cl: Add "/clang:" pass-through arg
support.".
neerajksingh edited the summary of this revision.
neerajksingh added a comment.
Change the spelling of from "/Xdriver:" to "/clang:".
Refactor getIncludeExcludeOptionFlagMasks to take whether we're in CL mode as
an argument, making the function pure.
https://reviews.llvm.org/D53457
Files:
include/clang/Driver/CLCompatOptions.td
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
test/Driver/cl-options.c
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -614,5 +614,17 @@
// RUN: --version \
// RUN: -Werror /Zs -- %s 2>&1
+// Accept clang options under the /clang: flag.
+
+// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s
+// NOCLANG: "--dependent-lib=libcmt"
+// NOCLANG-SAME: "-vectorize-slp"
+// NOCLANG-NOT: "--dependent-lib=msvcrt"
+
+// RUN: %clang_cl -O2 -MD /clang:-fno-slp-vectorize /clang:-MD /clang:-MF /clang:my_dependency_file.dep -### -- %s 2>&1 | FileCheck -check-prefix=CLANG %s
+// CLANG: "--dependent-lib=msvcrt"
+// CLANG-SAME: "-dependency-file" "my_dependency_file.dep"
+// CLANG-NOT: "--dependent-lib=libcmt"
+// CLANG-NOT: "-vectorize-slp"
void f() { }
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -166,14 +166,15 @@
}
InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings,
+ bool IsClCompatMode,
bool ) {
llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
ContainsError = false;
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+ getIncludeExcludeOptionFlagMasks(IsClCompatMode);
unsigned MissingArgIndex, MissingArgCount;
InputArgList Args =
@@ -730,7 +731,7 @@
ConfigFile = CfgFileName.str();
bool ContainErrors;
CfgOptions = llvm::make_unique(
- ParseArgStrings(NewCfgArgs, ContainErrors));
+ ParseArgStrings(NewCfgArgs, IsCLMode(), ContainErrors));
if (ContainErrors) {
CfgOptions.reset();
return true;
@@ -924,7 +925,7 @@
// Arguments specified in command line.
bool ContainsError;
CLOptions = llvm::make_unique(
- ParseArgStrings(ArgList.slice(1), ContainsError));
+ ParseArgStrings(ArgList.slice(1), IsCLMode(), ContainsError));
// Try parsing configuration file.
if (!ContainsError)
@@ -934,21 +935,47 @@
// All arguments, from both config file and command line.
InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
: std::move(*CLOptions));
- if (HasConfigFile)
-for (auto *Opt : *CLOptions) {
- if (Opt->getOption().matches(options::OPT_config))
-continue;
+
+ auto appendOneArg = [](const Arg *Opt, const Arg *BaseArg) {
unsigned Index = Args.MakeIndex(Opt->getSpelling());
- const Arg *BaseArg = >getBaseArg();
- if (BaseArg == Opt)
-BaseArg = nullptr;
Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Opt->getSpelling(),
Index, BaseArg);
Copy->getValues() = Opt->getValues();
if (Opt->isClaimed())
Copy->claim();
Args.append(Copy);
+ };
+
+ if (HasConfigFile)
+for (auto *Opt : *CLOptions) {
+ if (Opt->getOption().matches(options::OPT_config))
+continue;
+ const Arg *BaseArg = >getBaseArg();
+ if (BaseArg == Opt)
+BaseArg = nullptr;
+ appendOneArg(Opt, BaseArg);
+}
+
+ // In CL mode, look for any pass-through arguments
+ if (IsCLMode() && !ContainsError) {
+SmallVector CLModePassThroughArgList;
+for (const auto *A : Args.filtered(options::OPT__SLASH_clang)) {
+ A->claim();
+ CLModePassThroughArgList.push_back(A->getValue());
+}
+
+if (!CLModePassThroughArgList.empty()) {
+ // Parse any pass through args using default clang processing rather
+ // than clang-cl processing.
+ auto CLModePassThroughOptions = llvm::make_unique(
+ ParseArgStrings(CLModePassThroughArgList, false, ContainsError));
+
+ if (!ContainsError)
+for (auto *Opt : *CLModePassThroughOptions) {
+ appendOneArg(Opt, nullptr);
+}
}
+ }
// FIXME: This stuff needs to go into the Compilation, not the driver.
bool CCCPrintPhases;
@@ -1452,7 +1479,7 @@
unsigned IncludedFlagsBitmask;
unsigned ExcludedFlagsBitmask;
std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
- getIncludeExcludeOptionFlagMasks();
+
