[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D103461#2862067 , @jansvoboda11 
wrote:

> In D103461#2841918 , @bmahjour 
> wrote:
>
>> @jansvoboda11 This change is causing the following LIT tests to fail on AIX:
>>
>>   Clang :: ClangScanDeps/headerwithdirname.cpp
>>   Clang :: ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
>>
>> The reason seems to be related to the fact that `-fno-integrated-as` is on 
>> by default on that platform. I get the same failure on Linux if I change the 
>> "compilation database" file to add `-fno-integrated-as` to the "command" 
>> line:
>>
>>   > /build_llvm/bin/clang-scan-deps -compilation-database 
>> ./headerwithdirname.cpp.tmp.cdb -j 1
>>   > Error while scanning dependencies for 
>> /build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/headerwithdirname_input.cpp:
>>   error: unable to handle compilation, expected exactly one compiler job in 
>> ' "clang" "-cc1" "-triple" "powerpc64le-unknown-linux-gnu" "-S" ...;  
>> "/usr/bin/as" "-a64" "-mppc64" "-mlittle-endian" "-mpower8" "-I" 
>> "/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir"
>>  "-I" 
>> "/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/foodir"
>>  "-I" "Inputs" "-o" "headerwithdirname_input.o" 
>> "/tmp/headerwithdirname_input-2e0050.s"; '
>
> Thanks for the report and the reproducer. I'll try to get a fix ready 
> tomorrow.

Fixed here: https://reviews.llvm.org/D105695


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103461/new/

https://reviews.llvm.org/D103461

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

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D103461#2841918 , @bmahjour wrote:

> @jansvoboda11 This change is causing the following LIT tests to fail on AIX:
>
>   Clang :: ClangScanDeps/headerwithdirname.cpp
>   Clang :: ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
>
> The reason seems to be related to the fact that `-fno-integrated-as` is on by 
> default on that platform. I get the same failure on Linux if I change the 
> "compilation database" file to add `-fno-integrated-as` to the "command" line:
>
>   > /build_llvm/bin/clang-scan-deps -compilation-database 
> ./headerwithdirname.cpp.tmp.cdb -j 1
>   > Error while scanning dependencies for 
> /build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/headerwithdirname_input.cpp:
>   error: unable to handle compilation, expected exactly one compiler job in ' 
> "clang" "-cc1" "-triple" "powerpc64le-unknown-linux-gnu" "-S" ...;  
> "/usr/bin/as" "-a64" "-mppc64" "-mlittle-endian" "-mpower8" "-I" 
> "/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir"
>  "-I" 
> "/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/foodir"
>  "-I" "Inputs" "-o" "headerwithdirname_input.o" 
> "/tmp/headerwithdirname_input-2e0050.s"; '

Thanks for the report and the reproducer. I'll try to get a fix ready tomorrow.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103461/new/

https://reviews.llvm.org/D103461

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

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D103461#2841918 , @bmahjour wrote:

> @jansvoboda11 This change is causing the following LIT tests to fail on AIX:

I think @jansvoboda11 is out this week (IIRC, back next week). A possible 
interim workaround would be to add an `XFAIL: aix` (or whatever the lit feature 
is for AIX triples)... are you able to land something like that to unblock you? 
(Seems like the correct fix for `-fno-integrated-as` could be a bit more 
complicated...)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103461/new/

https://reviews.llvm.org/D103461

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

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added a comment.

@jansvoboda11 This change is causing the following LIT tests to fail on AIX:

  Clang :: ClangScanDeps/headerwithdirname.cpp
  Clang :: ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp

The reason seems to be related to the fact that `-fno-integrated-as` is on by 
default on that platform. I get the same failure on Linux if I change the 
"compilation database" file to add `-fno-integrated-as` to the "command" line:

  > ./bin/clang-scan-deps -compilation-database ./headerwithdirname.cpp.tmp.cdb 
-j 1
  > Error while scanning dependencies for 
/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/headerwithdirname_input.cpp:
  error: unable to handle compilation, expected exactly one compiler job in ' 
"clang" "-cc1" "-triple" "powerpc64le-unknown-linux-gnu" "-S" ...;  
"/usr/bin/as" "-a64" "-mppc64" "-mlittle-endian" "-mpower8" "-I" 
"/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir"
 "-I" 
"/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/foodir"
 "-I" "Inputs" "-o" "headerwithdirname_input.o" 
"/tmp/headerwithdirname_input-2e0050.s"; '


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103461/new/

https://reviews.llvm.org/D103461

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

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D103461#2815913 , @dexonsmith 
wrote:

> Okay, LGTM. (Sorry for the delay, I've been out.)
>
> In D103461#2793254 , @jansvoboda11 
> wrote:
>
>> My reason for the FIXME is that we could get rid of bunch of 
>> Windows-specific logic by adjusting `CompilerInvocation` instead.
>
> Please add that motivation to the FIXME itself.

No problem, thanks for the review(s)!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103461/new/

https://reviews.llvm.org/D103461

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

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG669771cfe75b: [clang][deps] NFC: Preserve the original 
frontend action (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D103461?vs=349229=351795#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103461/new/

https://reviews.llvm.org/D103461

Files:
  clang/tools/clang-scan-deps/ClangScanDeps.cpp


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -462,6 +462,10 @@
   std::make_unique(
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
+
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  // instead of parsing and adjusting the raw command-line. This will make it
+  // possible to remove some code specific to clang-cl and Windows.
   AdjustingCompilations->appendArgumentsAdjuster(
   [](const tooling::CommandLineArguments ,
   StringRef FileName) {
@@ -532,7 +536,7 @@
 #else
 AdjustedArgs.push_back("/dev/null");
 #endif
-if (!HasMT && !HasMQ) {
+if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
   // We're interested in source dependencies of an object file.
   std::string FileNameArg;
   if (!HasMD) {
@@ -553,8 +557,6 @@
   }
 }
 AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-Eonly");
-AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
 AdjustedArgs.push_back("-Wno-error");
 


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -462,6 +462,10 @@
   std::make_unique(
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
+
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  // instead of parsing and adjusting the raw command-line. This will make it
+  // possible to remove some code specific to clang-cl and Windows.
   AdjustingCompilations->appendArgumentsAdjuster(
   [](const tooling::CommandLineArguments ,
   StringRef FileName) {
@@ -532,7 +536,7 @@
 #else
 AdjustedArgs.push_back("/dev/null");
 #endif
-if (!HasMT && !HasMQ) {
+if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
   // We're interested in source dependencies of an object file.
   std::string FileNameArg;
   if (!HasMD) {
@@ -553,8 +557,6 @@
   }
 }
 AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-Eonly");
-AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
 AdjustedArgs.push_back("-Wno-error");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

Okay, LGTM. (Sorry for the delay, I've been out.)

In D103461#2793254 , @jansvoboda11 
wrote:

> My reason for the FIXME is that we could get rid of bunch of Windows-specific 
> logic by adjusting `CompilerInvocation` instead.

Please add that motivation to the FIXME itself.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103461/new/

https://reviews.llvm.org/D103461

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