This is an archive of the discontinued LLVM Phabricator instance.

Differential D30743

enable -save-temps with -finclude-defult-header
ClosedPublic

Authored by guansong on Mar 8 2017, 9:05 AM.

Details

Reviewers
yaxunl
Anastasia
Commits
rG4747cf5677d2: enable -save-temps with -finclude-defult-header
rC297890: enable -save-temps with -finclude-defult-header
rL297890: enable -save-temps with -finclude-defult-header
Summary

Currently the two flags can not work together.

To illustrate the issue, we can have an one line file a.cl contains only an empty function

cat a.cl

void test(){}

Then use

clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang -finclude-default-header -target amdgcn -S -c a.cl

we will get redefinition errors for various things.

The reason is that the -finclude-default-header flag is not meant to be on cc1 command other than the preprocessor.

The fix is modeled after the code just below the change to filter the -finclude-default-header flag out when we are not in the preprocess phase.

Diff Detail

Repository
rL LLVM

Event Timeline

guansong created this revision.Mar 8 2017, 9:05 AM
guansong added a reviewer: yaxunl.
guansong updated this revision to Diff 91045.Mar 8 2017, 10:11 AM

add a test

guansong added a reviewer: Anastasia.Mar 8 2017, 10:13 AM
guansong added a subscriber: cfe-commits.
Anastasia added inline comments.Mar 13 2017, 10:50 AM
lib/Driver/Tools.cpp
5288 ↗(On Diff #91045)

It would be nice to add a comment around here explaining this case.

5290 ↗(On Diff #91045)

Remove the commented code, please!

guansong updated this revision to Diff 91600.Mar 13 2017, 11:54 AM
Anastasia accepted this revision.Mar 14 2017, 7:08 AM

LGTM! Thanks!

lib/Driver/Tools.cpp
5290 ↗(On Diff #91045)

I think the comment could be lifted up before the if statement. :)

This revision is now accepted and ready to land.Mar 14 2017, 7:08 AM
guansong updated this revision to Diff 91718.Mar 14 2017, 7:21 AM

Thanks, update as suggested.

Closed by commit rL297890: enable -save-temps with -finclude-defult-header (authored by guansong). · Explain WhyMar 15 2017, 2:11 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
cfe/
trunk/
lib/
Driver/
ToolChains/
14 lines
test/
Driver/
4 lines

Diff 91935

cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Show First 20 Lines • Show All 4,262 Lines • ▼ Show 20 Lines if (DoSaveStats) {
llvm::sys::path::replace_extension(StatsFile, "stats"); llvm::sys::path::replace_extension(StatsFile, "stats");
CmdArgs.push_back(Args.MakeArgString(Twine("-stats-file=") + CmdArgs.push_back(Args.MakeArgString(Twine("-stats-file=") +
StatsFile)); StatsFile));
} }
} }
// Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
// parser. // parser.
// -finclude-default-header flag is for preprocessor,
// do not pass it to other cc1 commands when save-temps is enabled
if (C.getDriver().isSaveTempsEnabled() &&
!isa<PreprocessJobAction>(JA)) {
for (auto Arg : Args.filtered(options::OPT_Xclang)) {
Arg->claim();
if (StringRef(Arg->getValue()) != "-finclude-default-header")
CmdArgs.push_back(Arg->getValue());
}
}
else {
Args.AddAllArgValues(CmdArgs, options::OPT_Xclang); Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
}
for (const Arg *A : Args.filtered(options::OPT_mllvm)) { for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
A->claim(); A->claim();
// We translate this by hand to the -cc1 argument, since nightly test uses // We translate this by hand to the -cc1 argument, since nightly test uses
// it and developers have been trained to spell it with -mllvm. Both // it and developers have been trained to spell it with -mllvm. Both
// spellings are now deprecated and should be removed. // spellings are now deprecated and should be removed.
if (StringRef(A->getValue(0)) == "-disable-llvm-optzns") { if (StringRef(A->getValue(0)) == "-disable-llvm-optzns") {
CmdArgs.push_back("-disable-llvm-optzns"); CmdArgs.push_back("-disable-llvm-optzns");
▲ Show 20 Lines • Show All 898 Lines • Show Last 20 Lines

cfe/trunk/test/Driver/include-default-header.cl

// RUN: %clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang -finclude-default-header -target amdgcn -S -c %s
void test() {}