[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-23 Thread Fangrui Song via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc2fb114475d1: [Driver] Enable getOSLibDir() lib32 workaround 
for SPARC on Linux (authored by glaubitz, committed by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D90524?vs=303846&id=307233#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/debian_8_sparc64_tree/lib/sparc64-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_8_sparc64_tree/lib64/.keep
  clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/include/c++/4.9/.keep
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/include/sparc64-linux-gnu/c++/4.9/.keep
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/gcc/sparc64-linux-gnu/4.9/crtbegin.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/gcc/sparc64-linux-gnu/4.9/crtend.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/sparc64-linux-gnu/crt1.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/sparc64-linux-gnu/crti.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/sparc64-linux-gnu/crtn.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/lib/sparc-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/lib64/.keep
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/include/c++/4.9/backward/.keep
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/include/sparc-linux-gnu/c++/4.9/64/.keep
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/64/crtbegin.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/64/crtend.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/crtbegin.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/crtend.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/sparc-linux-gnu/crt1.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/sparc-linux-gnu/crti.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/sparc-linux-gnu/crtn.o
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib64/crt1.o
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib64/crti.o
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib64/crtn.o
  clang/test/Driver/Inputs/debian_multiarch_tree/lib/sparc-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_multiarch_tree/lib/sparc64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/c++/4.5/sparc-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/c++/4.5/sparc64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/sparc-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/sparc64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/gcc/sparc-linux-gnu/4.5/crtbegin.o
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/gcc/sparc64-linux-gnu/4.5/crtbegin.o
  clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/sparc-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/sparc64-linux-gnu/.keep
  clang/test/Driver/linux-header-search.cpp
  clang/test/Driver/linux-ld.c

Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -1282,67 +1282,32 @@
 // CHECK-DEBIAN-MIPS64EL-N32: "-L[[SYSROOT]]/usr/lib/gcc/mipsel-linux-gnu/4.5/../../.."
 // CHECK-DEBIAN-MIPS64EL-N32: "-L[[SYSROOT]]/lib"
 // CHECK-DEBIAN-MIPS64EL-N32: "-L[[SYSROOT]]/usr/lib"
-//
-// Check linker paths on Debian 8 / Sparc
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=sparc-linux-gnu -rtlib=platform \
 // RUN: --gcc-toolchain="" \
-// RUN: --sysroot=%S/Inputs/debian_8_sparc_multilib_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-DEBIAN-SPARC32 %s
-// CHECK-DEBIAN-SPARC32: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../sparc-linux-gnu{{/|}}crt1.o"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../sparc-linux-gnu{{/|}}crti.o"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9{{/|}}crtbegin.o"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../sparc-linux-gnu"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../../lib"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/lib/sparc-linux-gnu"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/sparc-linux-gnu"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/lib"

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-23 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: clang/test/Driver/linux-header-search.cpp:266
+// CHECK-DEBIAN-SPARC: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-DEBIAN-SPARC: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-DEBIAN-SPARC: "-isysroot" "[[SYSROOT:[^"]+]]"

I'll add some -SAME directives because that helps FileCheck to provide a 
friendly error message when things go off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-23 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

Thanks so much! Would you mind pushing that change for me? I don't have commit 
access at the moment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-23 Thread Petr Hosek via Phabricator via cfe-commits 
phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-23 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

I changed it to 4.5 to be consisted with the other Multi-Arch tests for x86, 
MIPS and PowerPC. That's all.

Debian Multi-Arch on sparc/sparc64 behaves the exact same way as it does on 
mips*, powerpc* and x86, so I think it makes sense to use the exact same path 
patterns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-23 Thread Petr Hosek via Phabricator via cfe-commits 
phosek added inline comments.



Comment at: clang/test/Driver/linux-ld.c:1292
+// CHECK-DEBIAN-SPARC: 
"{{.*}}/usr/lib/gcc/sparc-linux-gnu/4.5{{/|}}crtbegin.o"
+// CHECK-DEBIAN-SPARC: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.5"
+// CHECK-DEBIAN-SPARC: 
"-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.5/../../../sparc-linux-gnu"

What's the reason for changing the version from 4.9 to 4.5? Not that it really 
matters, but I'm curious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread Rainer Orth via Phabricator via cfe-commits 
ro added a comment.

In D90524#2393747 , @glaubitz wrote:

> 



> No worries. I'm glad someone is taking care of the Solaris parts and 
> appreciate fixes from other parties.

My pleasure: hopefully our work will also be for the benefit of the other 
party.  I certainly do have some patches in the queue that will fix bugs also 
occuring on Linux/SPARC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

In D90524#2393746 , @ro wrote:

> In D90524#2393651 , @glaubitz wrote:
>
>> 
>
>
>
>> Please feel free to reach out to the corresponding debian-$ARCH mailing list 
>> for such questions. Debian-specific layouts are not
>> necessarily obvious at first glance to people not very familiar with the 
>> distribution.
>
> I could have.  However, my only interest in Linux/SPARC was to have a 
> comparison point for my Solaris/SPARC work to determine which test failures 
> occur on all SPARC targets and which ones are Solaris-specific.  I simply 
> don't have to time to dig deeper into Linux issues.

No worries. I'm glad someone is taking care of the Solaris parts and appreciate 
fixes from other parties.

Please don't see my comments as dismissive in any way, they aren't meant that 
way. I appreciate the feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread Rainer Orth via Phabricator via cfe-commits 
ro added a comment.

In D90524#2393651 , @glaubitz wrote:

> 



> Please feel free to reach out to the corresponding debian-$ARCH mailing list 
> for such questions. Debian-specific layouts are not
> necessarily obvious at first glance to people not very familiar with the 
> distribution.

I could have.  However, my only interest in Linux/SPARC was to have a 
comparison point for my Solaris/SPARC work to determine which test failures 
occur on all SPARC targets and which ones are Solaris-specific.  I simply don't 
have to time to dig deeper into Linux issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

> ! In D90524#2393635 , @ro wrote:
>
> I had an extremely hard time researching the history of directory layouts for 
> my patch D85582 .  Do as you like, I'm out 
> of this.

Please feel free to reach out to the corresponding debian-$ARCH mailing list 
for such questions. Debian-specific layouts are not
necessarily obvious at first glance to people not very familiar with the 
distribution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread Rainer Orth via Phabricator via cfe-commits 
ro added a comment.

In D90524#2393366 , @glaubitz wrote:

> In D90524#2393320 , @ro wrote:
>
>> In D90524#2393312 , @glaubitz wrote:
>>
>>> Ping. It would be nice to get this finally merged so that the testsuite 
>>> noise finally goes down on the sparc64 Linux worker.
>>
>> Please be a little more patient: one ping a week is considered appropriate, 
>> but after only two days is a bit over the top.
>
> The problem is that LLVM is a very fast moving target and when waiting long 
> for changes to be merged, one constantly runs
> into the risk of having to rebase patches.

True, but it's the same for everyone.  Constantly pinging at short intervals 
won't get you faster reviews, but just creates noise and annoys possible 
reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread Rainer Orth via Phabricator via cfe-commits 
ro resigned from this revision.
ro added a comment.

In D90524#2393353 , @glaubitz wrote:

> In D90524#2393319 , @ro wrote:
>
>> In D90524#2388214 , @glaubitz wrote:
>>
>>> I think it should be good for merging now. I addressed all remarks. I'm 
>>> still convinced that "workaround" is the proper term though.
>>
>> Quite the contrary: the comment you cited
>>
>>   // FIXME: This is a bit of a hack. We should really unify this code for
>>   // reasoning about oslibdir spellings with the lib dir spellings in the
>>   // GCCInstallationDetector, but that is a more significant refactoring.
>>
>> pretty clearly is about how/where support for that layout is implemented in 
>> the `clang` Driver code, not about the layout itself.
>
> I don't understand that argument. I call it "workaround", the source comment 
> calls it "hack". It's clearly not to stay forever as it's an ugly
> workaround, but until a proper fix comes around, I would like to add "sparc" 
> here as well so the testsuite failures drop from over
> 400 to just below 70.

I think we're talking past each other, but as I've said, i'm not able to review 
the rest of this patch, so I won't belabour the point.

>> Besides, you haven't explained why it's appropriate to no longer test 
>> support for the old (pre-Debian 9,I believe) directory layout.  However, as 
>> I said I don't feel qualified to review that part, so you'll need another 
>> reviewer for that, no matter if only testing the new layout or both old and 
>> new ones.
>
> Debian 8 doesn't even support sparc as the port was dropped in this release:
>
>> https://cdimage.debian.org/cdimage/archive/8.0.0/
>
> The last sparc release was 7.11.0 and that's Wheezy which is long out of 
> support:
>
>> https://cdimage.debian.org/cdimage/archive/7.11.0/
>
> And, as I said, MultiArch was and is the same for all architectures, 
> including sparc/sparc64. It does not make sense to test sparc here 
> differently than the other
> Debian architectures. There is no special sparc configuration in Debian and I 
> think I can make that statement as Debian's primary maintainer for the sparc64
> port.

I had an extremely hard time researching the history of directory layouts for 
my patch D85582 .  Do as you like, I'm out of 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

In D90524#2393320 , @ro wrote:

> In D90524#2393312 , @glaubitz wrote:
>
>> Ping. It would be nice to get this finally merged so that the testsuite 
>> noise finally goes down on the sparc64 Linux worker.
>
> Please be a little more patient: one ping a week is considered appropriate, 
> but after only two days is a bit over the top.

The problem is that LLVM is a very fast moving target and when waiting long for 
changes to be merged, one constantly runs
into the risk of having to rebase patches.

I would like to move forward with other changes and having unmerged changes 
open takes away attention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

In D90524#2393319 , @ro wrote:

> In D90524#2388214 , @glaubitz wrote:
>
>> I think it should be good for merging now. I addressed all remarks. I'm 
>> still convinced that "workaround" is the proper term though.
>
> Quite the contrary: the comment you cited
>
>   // FIXME: This is a bit of a hack. We should really unify this code for
>   // reasoning about oslibdir spellings with the lib dir spellings in the
>   // GCCInstallationDetector, but that is a more significant refactoring.
>
> pretty clearly is about how/where support for that layout is implemented in 
> the `clang` Driver code, not about the layout itself.

I don't understand that argument. I call it "workaround", the source comment 
calls it "hack". It's clearly not to stay forever as it's an ugly
workaround, but until a proper fix comes around, I would like to add "sparc" 
here as well so the testsuite failures drop from over
400 to just below 70.

> Besides, you haven't explained why it's appropriate to no longer test support 
> for the old (pre-Debian 9,I believe) directory layout.  However, as I said I 
> don't feel qualified to review that part, so you'll need another reviewer for 
> that, no matter if only testing the new layout or both old and new ones.

Debian 8 doesn't even support sparc as the port was dropped in this release:

> https://cdimage.debian.org/cdimage/archive/8.0.0/

The last sparc release was 7.11.0 and that's Wheezy which is long out of 
support:

> https://cdimage.debian.org/cdimage/archive/7.11.0/

And, as I said, MultiArch was and is the same for all architectures, including 
sparc/sparc64. It does not make sense to test sparc here differently than the 
other
Debian architectures. There is no special sparc configuration in Debian and I 
think I can make that statement as Debian's primary maintainer for the sparc64
port.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread Rainer Orth via Phabricator via cfe-commits 
ro added a comment.

In D90524#2393312 , @glaubitz wrote:

> Ping. It would be nice to get this finally merged so that the testsuite noise 
> finally goes down on the sparc64 Linux worker.

Please be a little more patient: one ping a week is considered appropriate, but 
after only two days is a bit over the top.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread Rainer Orth via Phabricator via cfe-commits 
ro added a comment.

In D90524#2388214 , @glaubitz wrote:

> I think it should be good for merging now. I addressed all remarks. I'm still 
> convinced that "workaround" is the proper term though.

Quite the contrary: the comment you cited

  // FIXME: This is a bit of a hack. We should really unify this code for
  // reasoning about oslibdir spellings with the lib dir spellings in the
  // GCCInstallationDetector, but that is a more significant refactoring.

pretty clearly is about how/where support for that layout is implemented in the 
`clang` Driver code, not about the layout itself.

Besides, you haven't explained why it's appropriate to no longer test support 
for the old (pre-Debian 9,I believe) directory layout.  However, as I said I 
don't feel qualified to review that part, so you'll need another reviewer for 
that, no matter if only testing the new layout or both old and new ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-13 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

Ping. It would be nice to get this finally merged so that the testsuite noise 
finally goes down on the sparc64 Linux worker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-11 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

I think it should be good for merging now. I addressed all remarks. I'm still 
convinced that "workaround" is the proper term though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-09 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz updated this revision to Diff 303846.
glaubitz edited the summary of this revision.
glaubitz added a comment.
Herald added subscribers: steven.zhang, jrtc27.

Update as requested in previous review, also merge with D90549 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/debian_8_sparc64_tree/lib/sparc64-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_8_sparc64_tree/lib64/.keep
  clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/include/c++/4.9/.keep
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/include/sparc64-linux-gnu/c++/4.9/.keep
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/gcc/sparc64-linux-gnu/4.9/crtbegin.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/gcc/sparc64-linux-gnu/4.9/crtend.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/sparc64-linux-gnu/crt1.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/sparc64-linux-gnu/crti.o
  
clang/test/Driver/Inputs/debian_8_sparc64_tree/usr/lib/sparc64-linux-gnu/crtn.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/lib/sparc-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/lib64/.keep
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/include/c++/4.9/backward/.keep
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/include/sparc-linux-gnu/c++/4.9/64/.keep
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/64/crtbegin.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/64/crtend.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/crtbegin.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/gcc/sparc-linux-gnu/4.9/crtend.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/sparc-linux-gnu/crt1.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/sparc-linux-gnu/crti.o
  
clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib/sparc-linux-gnu/crtn.o
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib64/crt1.o
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib64/crti.o
  clang/test/Driver/Inputs/debian_8_sparc_multilib_tree/usr/lib64/crtn.o
  clang/test/Driver/Inputs/debian_multiarch_tree/lib/sparc-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_multiarch_tree/lib/sparc64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/c++/4.5/sparc-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/c++/4.5/sparc64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/sparc-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/include/sparc64-linux-gnu/.keep
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/gcc/sparc-linux-gnu/4.5/crtbegin.o
  
clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/gcc/sparc64-linux-gnu/4.5/crtbegin.o
  clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/sparc-linux-gnu/.keep
  clang/test/Driver/Inputs/debian_multiarch_tree/usr/lib/sparc64-linux-gnu/.keep
  clang/test/Driver/linux-header-search.cpp
  clang/test/Driver/linux-ld.c

Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -1282,67 +1282,32 @@
 // CHECK-DEBIAN-MIPS64EL-N32: "-L[[SYSROOT]]/usr/lib/gcc/mipsel-linux-gnu/4.5/../../.."
 // CHECK-DEBIAN-MIPS64EL-N32: "-L[[SYSROOT]]/lib"
 // CHECK-DEBIAN-MIPS64EL-N32: "-L[[SYSROOT]]/usr/lib"
-//
-// Check linker paths on Debian 8 / Sparc
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=sparc-linux-gnu -rtlib=platform \
 // RUN: --gcc-toolchain="" \
-// RUN: --sysroot=%S/Inputs/debian_8_sparc_multilib_tree \
-// RUN:   | FileCheck --check-prefix=CHECK-DEBIAN-SPARC32 %s
-// CHECK-DEBIAN-SPARC32: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../sparc-linux-gnu{{/|}}crt1.o"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../sparc-linux-gnu{{/|}}crti.o"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9{{/|}}crtbegin.o"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../sparc-linux-gnu"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/gcc/sparc-linux-gnu/4.9/../../../../lib"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/lib/sparc-linux-gnu"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib/sparc-linux-gnu"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/lib"
-// CHECK-DEBIAN-SPARC32: "-L[[SYSROOT]]/usr/lib"
-// CHECK-DEBIAN-SPARC32: "[[SYSROOT]]/usr

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-09 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

Hi Rainer!

Thanks for your review.

In D90524#2382555 , @ro wrote:

> As it happens, I'd arrived at exactly the same patch when I tried a build on 
> a Debian/sparc64 system in the GCC compile farm (gcc202), submitted as D85582 
> .
>
> A couple of questions/caveats:
>
> - I wouldn't call this patch a workaround, as you do in the summary: if 
> `clang` cannot find the 32-bit libs, it's broken and needs to be fixed.

I'm calling it a workaround because it's already called like that in the 
existing code comment (well, they call it "hack"):

> // FIXME: This is a bit of a hack. We should really unify this code for
> // reasoning about oslibdir spellings with the lib dir spellings in the
> // GCCInstallationDetector, but that is a more significant refactoring.



> - Please reformat `Linux.cpp` as the pre-merge check requested.

OK.

> - I'd be willing to accept this patch, but for one I'm not certain about LLVM 
> policy about who may or may not do so.
> - Besides, I believe it's a mistake to split the bug fix and the testsuite 
> change into to different patches.  More about that in D90549 
> .

OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-09 Thread Rainer Orth via Phabricator via cfe-commits 
ro added a comment.

As it happens, I'd arrived at exactly the same patch when I tried a build on a 
Debian/sparc64 system in the GCC compile farm (gcc202), submitted as D85582 
.

A couple of questions/caveats:

- I wouldn't call this patch a workaround, as you do in the summary: if `clang` 
cannot find the 32-bit libs, it's broken and needs to be fixed.
- Please reformat `Linux.cpp` as the pre-merge check requested.
- I'd be willing to accept this patch, but for one I'm not certain about LLVM 
policy about who may or may not do so.
- Besides, I believe it's a mistake to split the bug fix and the testsuite 
change into to different patches.  More about that in D90549 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-09 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-11-01 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

The tests are fixed by this change: https://reviews.llvm.org/D90549.

The tests CHECK-DEBIAN-SPARC32 is outdated and does not reflect the fact that 
sparc and sparc64 use MultiArch on Debian as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-10-31 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz added a comment.

Hmm, I'm not sure why the CHECK-DEBIAN-SPARC32 test is failing but I assume the 
expected output needs to be updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90524

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

[PATCH] D90524: [Driver] Enable getOSLibDir() lib32 workaround for SPARC on Linux

2020-10-31 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits 
glaubitz created this revision.
glaubitz added a reviewer: nemanjai.
Herald added subscribers: cfe-commits, pengfei, fedor.sergeev, jyknight.
Herald added a project: clang.
glaubitz requested review of this revision.

This fixes the Builtins-sparc-linux testsuite failures on Linux
SPARC which occur because clang cannot find the 32-bit runtime
libraries when -m32 is passed on the command line.

The same workaround is already being used on X86 and PPC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90524

Files:
  clang/lib/Driver/ToolChains/Linux.cpp


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -185,8 +185,8 @@
 return Triple.isArch32Bit() ? "lib" : "lib64";
   }
 
-  // It happens that only x86 and PPC use the 'lib32' variant of oslibdir, and
-  // using that variant while targeting other architectures causes problems
+  // It happens that only x86, PPC and SPARC use the 'lib32' variant of 
oslibdir,
+  // and using that variant while targeting other architectures causes problems
   // because the libraries are laid out in shared system roots that can't cope
   // with a 'lib32' library search path being considered. So we only enable
   // them when we know we may need it.
@@ -195,7 +195,8 @@
   // reasoning about oslibdir spellings with the lib dir spellings in the
   // GCCInstallationDetector, but that is a more significant refactoring.
   if (Triple.getArch() == llvm::Triple::x86 ||
-  Triple.getArch() == llvm::Triple::ppc)
+  Triple.getArch() == llvm::Triple::ppc ||
+  Triple.getArch() == llvm::Triple::sparc)
 return "lib32";
 
   if (Triple.getArch() == llvm::Triple::x86_64 &&


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -185,8 +185,8 @@
 return Triple.isArch32Bit() ? "lib" : "lib64";
   }
 
-  // It happens that only x86 and PPC use the 'lib32' variant of oslibdir, and
-  // using that variant while targeting other architectures causes problems
+  // It happens that only x86, PPC and SPARC use the 'lib32' variant of oslibdir,
+  // and using that variant while targeting other architectures causes problems
   // because the libraries are laid out in shared system roots that can't cope
   // with a 'lib32' library search path being considered. So we only enable
   // them when we know we may need it.
@@ -195,7 +195,8 @@
   // reasoning about oslibdir spellings with the lib dir spellings in the
   // GCCInstallationDetector, but that is a more significant refactoring.
   if (Triple.getArch() == llvm::Triple::x86 ||
-  Triple.getArch() == llvm::Triple::ppc)
+  Triple.getArch() == llvm::Triple::ppc ||
+  Triple.getArch() == llvm::Triple::sparc)
 return "lib32";
 
   if (Triple.getArch() == llvm::Triple::x86_64 &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits