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

2017-03-15 Thread Guansong Zhang via cfe-commits 
Author: guansong
Date: Wed Mar 15 15:57:11 2017
New Revision: 297890

URL: http://llvm.org/viewvc/llvm-project?rev=297890&view=rev
Log:
enable -save-temps with -finclude-defult-header

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.

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

Added:
cfe/trunk/test/Driver/include-default-header.cl
Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=297890&r1=297889&r2=297890&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed Mar 15 15:57:11 2017
@@ -4268,7 +4268,19 @@ void Clang::ConstructJob(Compilation &C,
 
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
-  Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
+  // -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(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);
+  }
   for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
 A->claim();
 

Added: cfe/trunk/test/Driver/include-default-header.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/include-default-header.cl?rev=297890&view=auto
==
--- cfe/trunk/test/Driver/include-default-header.cl (added)
+++ cfe/trunk/test/Driver/include-default-header.cl Wed Mar 15 15:57:11 2017
@@ -0,0 +1,4 @@
+// RUN: %clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang 
-finclude-default-header -target amdgcn -S -c %s
+
+void test() {}
+


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r297896 - fix build break by removing the target on command line

2017-03-15 Thread Guansong Zhang via cfe-commits 
Author: guansong
Date: Wed Mar 15 16:46:44 2017
New Revision: 297896

URL: http://llvm.org/viewvc/llvm-project?rev=297896&view=rev
Log:
fix build break by removing the target on command line

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

Modified: cfe/trunk/test/Driver/include-default-header.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/include-default-header.cl?rev=297896&r1=297895&r2=297896&view=diff
==
--- cfe/trunk/test/Driver/include-default-header.cl (original)
+++ cfe/trunk/test/Driver/include-default-header.cl Wed Mar 15 16:46:44 2017
@@ -1,4 +1,4 @@
-// RUN: %clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang 
-finclude-default-header -target amdgcn -S -c %s
+// RUN: %clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang 
-finclude-default-header -emit-llvm -c %s
 
 void test() {}
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r297890 - enable -save-temps with -finclude-defult-header

2017-03-16 Thread Guansong Zhang via cfe-commits 
Many thanks!

Regards

Guansong

On Wed, Mar 15, 2017 at 7:55 PM, Eric Christopher 
wrote:

> Hi Guansong,
>
> So this wasn't quite the right way to write this testcase. You really
> wanted a CHECK/CHECK-NOT line along with the actual commands. You also
> didn't want to (as you noticed later) depend on full compilation. You can
> check for this sort of thing in the future by using the -### command line
> option to get the subcommands that clang -would- invoke if it were actually
> going to do it.
>
> I've gone ahead and fixed this thusly:
> Committing to https://llvm.org/svn/llvm-project/cfe/trunk ...
> M test/Driver/include-default-header.cl
> Committed r297917
>
> Thanks!
>
> -eric
>
> On Wed, Mar 15, 2017 at 2:09 PM Guansong Zhang via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: guansong
>> Date: Wed Mar 15 15:57:11 2017
>> New Revision: 297890
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=297890&view=rev
>> Log:
>> enable -save-temps with -finclude-defult-header
>>
>> 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.
>>
>> Differential Revision: https://reviews.llvm.org/D30743
>>
>> Added:
>> cfe/trunk/test/Driver/include-default-header.cl
>> Modified:
>> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>>
>> Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
>> ToolChains/Clang.cpp?rev=297890&r1=297889&r2=297890&view=diff
>> 
>> ==
>> --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
>> +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed Mar 15 15:57:11 2017
>> @@ -4268,7 +4268,19 @@ void Clang::ConstructJob(Compilation &C,
>>
>>// Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM
>> option
>>// parser.
>> -  Args.AddAllArgValues(CmdArgs, options::OPT_Xclang);
>> +  // -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(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);
>> +  }
>>for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
>>  A->claim();
>>
>>
>> Added: cfe/trunk/test/Driver/include-default-header.cl
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/
>> include-default-header.cl?rev=297890&view=auto
>> 
>> ==
>> --- cfe/trunk/test/Driver/include-default-header.cl (added)
>> +++ cfe/trunk/test/Driver/include-default-header.cl Wed Mar 15 15:57:11
>> 2017
>> @@ -0,0 +1,4 @@
>> +// RUN: %clang -v -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang
>> -finclude-default-header -target amdgcn -S -c %s
>> +
>> +void test() {}
>> +
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits