[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth 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 rGa6afa9e6b0d9: [Driver] Use libatomic for 32-bit SPARC 
atomics support (authored by ro).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/test/Driver/solaris-ld.c


Index: clang/test/Driver/solaris-ld.c
===
--- clang/test/Driver/solaris-ld.c
+++ clang/test/Driver/solaris-ld.c
@@ -17,6 +17,7 @@
 // CHECK-LD-SPARC32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2"
 // CHECK-LD-SPARC32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../.."
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-SPARC32-SAME: "-zignore" "-latomic" "-zrecord"
 // CHECK-LD-SPARC32-SAME: "-lgcc_s"
 // CHECK-LD-SPARC32-SAME: "-lc"
 // CHECK-LD-SPARC32-SAME: "-lgcc"
@@ -40,6 +41,7 @@
 // CHECK-LD-SPARC64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9"
 // CHECK-LD-SPARC64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../../sparcv9"
 // CHECK-LD-SPARC64-SAME: "-L[[SYSROOT]]/usr/lib/sparcv9"
+// CHECK-LD-SPARC64-NOT:  "-latomic"
 // CHECK-LD-SPARC64-SAME: "-lgcc_s"
 // CHECK-LD-SPARC64-SAME: "-lc"
 // CHECK-LD-SPARC64-SAME: "-lgcc"
@@ -63,6 +65,7 @@
 // CHECK-LD-X32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4"
 // CHECK-LD-X32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/../../.."
 // CHECK-LD-X32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-X32-NOT:  "-latomic"
 // CHECK-LD-X32-SAME: "-lgcc_s"
 // CHECK-LD-X32-SAME: "-lc"
 // CHECK-LD-X32-SAME: "-lgcc"
@@ -86,6 +89,7 @@
 // CHECK-LD-X64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/amd64"
 // CHECK-LD-X64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/../../../amd64"
 // CHECK-LD-X64-SAME: "-L[[SYSROOT]]/usr/lib/amd64"
+// CHECK-LD-X64-NOT:  "-latomic"
 // CHECK-LD-X64-SAME: "-lgcc_s"
 // CHECK-LD-X64-SAME: "-lc"
 // CHECK-LD-X64-SAME: "-lgcc"
Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+// forcibly link with libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -117,6 +117,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -756,7 +756,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 


Index: clang/test/Driver/solaris-ld.c
===
--- clang/test/Driver/solaris-ld.c
+++ clang/test/Driver/solaris-ld.c
@@ -17,6 +17,7 @@
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2"
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../.."
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-SPARC32-SAME: "-zignore" "-latomic" "-zrecord"
 // CHECK-LD-SPARC32-SAME: "-lgcc_s"
 // CHECK-LD-SPARC32-SAME: "-lc"
 // CHECK-LD-SPARC32-SAME: "-lgcc"
@@ -40,6 +41,7 @@
 // CHECK-LD-SPARC64-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9"
 // CHECK-LD-SP

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro added inline comments.



Comment at: clang/test/Driver/solaris-ld.c:46
 // CHECK-LD-SPARC64-SAME: "-L[[SYSROOT]]/usr/lib/sparcv9"
+// CHECK-LD-SPARC64-NOT:  "-zignore"
+// CHECK-LD-SPARC64-NOT:  "-latomic"

ro wrote:
> MaskRay wrote:
> > Such NOT patterns are usually inadequate and may go stale pretty easily, 
> > since technically the patterns can occur in many places.
> > 
> > One idea is to use --implicit-check-not; another is to enumerate all 
> > options and use the `{{^}}` style I picked in linux-cross.cpp, but perhaps 
> > your style is good enough if we can remember these library after after -L 
> > and before -lgcc_s
> > Such NOT patterns are usually inadequate and may go stale pretty easily, 
> > since technically the patterns can occur in many places.
> 
> I wondered so myself: while this is currently the only instance of 
> `-zignore`/`-zrecord`, `-lgcc_s` requires similar treatment.
> 
> > One idea is to use --implicit-check-not; another is to enumerate all 
> > options and use the `{{^}}` style I picked in linux-cross.cpp, but perhaps 
> > your style is good enough if we can remember these library after after -L 
> > and before -lgcc_s
> 
> I'll check those.  However, it occured to me that the crucial check is that 
> `-latomic` isn't added at all, `-zignore`/`-zrecord` or no, so maybe just 
> check for that and avoid the issue for the moment.
In the end, I've decided to go for the simple `*-NOT: "-latomic" form: it's 
simple and robust, which is all the more important since this patch needs to go 
into the `release/14.x` branch, too.

Going forward, I thing going for the `linux-cross.cpp` style is best.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 407455.
ro marked an inline comment as done.
ro added a comment.

Simplify test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/test/Driver/solaris-ld.c


Index: clang/test/Driver/solaris-ld.c
===
--- clang/test/Driver/solaris-ld.c
+++ clang/test/Driver/solaris-ld.c
@@ -17,6 +17,7 @@
 // CHECK-LD-SPARC32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2"
 // CHECK-LD-SPARC32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../.."
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-SPARC32-SAME: "-zignore" "-latomic" "-zrecord"
 // CHECK-LD-SPARC32-SAME: "-lgcc_s"
 // CHECK-LD-SPARC32-SAME: "-lc"
 // CHECK-LD-SPARC32-SAME: "-lgcc"
@@ -40,6 +41,7 @@
 // CHECK-LD-SPARC64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9"
 // CHECK-LD-SPARC64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../../sparcv9"
 // CHECK-LD-SPARC64-SAME: "-L[[SYSROOT]]/usr/lib/sparcv9"
+// CHECK-LD-SPARC64-NOT:  "-latomic"
 // CHECK-LD-SPARC64-SAME: "-lgcc_s"
 // CHECK-LD-SPARC64-SAME: "-lc"
 // CHECK-LD-SPARC64-SAME: "-lgcc"
@@ -63,6 +65,7 @@
 // CHECK-LD-X32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4"
 // CHECK-LD-X32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/../../.."
 // CHECK-LD-X32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-X32-NOT:  "-latomic"
 // CHECK-LD-X32-SAME: "-lgcc_s"
 // CHECK-LD-X32-SAME: "-lc"
 // CHECK-LD-X32-SAME: "-lgcc"
@@ -86,6 +89,7 @@
 // CHECK-LD-X64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/amd64"
 // CHECK-LD-X64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/../../../amd64"
 // CHECK-LD-X64-SAME: "-L[[SYSROOT]]/usr/lib/amd64"
+// CHECK-LD-X64-NOT:  "-latomic"
 // CHECK-LD-X64-SAME: "-lgcc_s"
 // CHECK-LD-X64-SAME: "-lc"
 // CHECK-LD-X64-SAME: "-lgcc"
Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+// forcibly link with libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -117,6 +117,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -756,7 +756,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 


Index: clang/test/Driver/solaris-ld.c
===
--- clang/test/Driver/solaris-ld.c
+++ clang/test/Driver/solaris-ld.c
@@ -17,6 +17,7 @@
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2"
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../.."
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-SPARC32-SAME: "-zignore" "-latomic" "-zrecord"
 // CHECK-LD-SPARC32-SAME: "-lgcc_s"
 // CHECK-LD-SPARC32-SAME: "-lc"
 // CHECK-LD-SPARC32-SAME: "-lgcc"
@@ -40,6 +41,7 @@
 // CHECK-LD-SPARC64-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9"
 // CHECK-LD-SPARC64-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../../sparcv9"
 // CHECK-LD-SPARC64-SAME: "-L

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro marked an inline comment as done.
ro added inline comments.



Comment at: clang/test/Driver/solaris-ld.c:20
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-SPARC32-SAME: "-zignore"
+// CHECK-LD-SPARC32-SAME: "-latomic"

MaskRay wrote:
> If they are consecutive.
The are, patch amended.



Comment at: clang/test/Driver/solaris-ld.c:46
 // CHECK-LD-SPARC64-SAME: "-L[[SYSROOT]]/usr/lib/sparcv9"
+// CHECK-LD-SPARC64-NOT:  "-zignore"
+// CHECK-LD-SPARC64-NOT:  "-latomic"

MaskRay wrote:
> Such NOT patterns are usually inadequate and may go stale pretty easily, 
> since technically the patterns can occur in many places.
> 
> One idea is to use --implicit-check-not; another is to enumerate all options 
> and use the `{{^}}` style I picked in linux-cross.cpp, but perhaps your style 
> is good enough if we can remember these library after after -L and before 
> -lgcc_s
> Such NOT patterns are usually inadequate and may go stale pretty easily, 
> since technically the patterns can occur in many places.

I wondered so myself: while this is currently the only instance of 
`-zignore`/`-zrecord`, `-lgcc_s` requires similar treatment.

> One idea is to use --implicit-check-not; another is to enumerate all options 
> and use the `{{^}}` style I picked in linux-cross.cpp, but perhaps your style 
> is good enough if we can remember these library after after -L and before 
> -lgcc_s

I'll check those.  However, it occured to me that the crucial check is that 
`-latomic` isn't added at all, `-zignore`/`-zrecord` or no, so maybe just check 
for that and avoid the issue for the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Driver/solaris-ld.c:20
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-SPARC32-SAME: "-zignore"
+// CHECK-LD-SPARC32-SAME: "-latomic"

If they are consecutive.



Comment at: clang/test/Driver/solaris-ld.c:46
 // CHECK-LD-SPARC64-SAME: "-L[[SYSROOT]]/usr/lib/sparcv9"
+// CHECK-LD-SPARC64-NOT:  "-zignore"
+// CHECK-LD-SPARC64-NOT:  "-latomic"

Such NOT patterns are usually inadequate and may go stale pretty easily, since 
technically the patterns can occur in many places.

One idea is to use --implicit-check-not; another is to enumerate all options 
and use the `{{^}}` style I picked in linux-cross.cpp, but perhaps your style 
is good enough if we can remember these library after after -L and before 
-lgcc_s


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 407120.
ro added a comment.

Add tests in `solaris-ld.c`.

Spot-checked by running the single test with `llvm-lit` on 
`sparcv9-sun-solaris2.11`,
`amd64-solaris2.11`, and `x86_64-pc-linux-gnu`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/test/Driver/solaris-ld.c


Index: clang/test/Driver/solaris-ld.c
===
--- clang/test/Driver/solaris-ld.c
+++ clang/test/Driver/solaris-ld.c
@@ -17,6 +17,9 @@
 // CHECK-LD-SPARC32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2"
 // CHECK-LD-SPARC32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../.."
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-SPARC32-SAME: "-zignore"
+// CHECK-LD-SPARC32-SAME: "-latomic"
+// CHECK-LD-SPARC32-SAME: "-zrecord"
 // CHECK-LD-SPARC32-SAME: "-lgcc_s"
 // CHECK-LD-SPARC32-SAME: "-lc"
 // CHECK-LD-SPARC32-SAME: "-lgcc"
@@ -40,6 +43,9 @@
 // CHECK-LD-SPARC64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/sparcv9"
 // CHECK-LD-SPARC64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../../sparcv9"
 // CHECK-LD-SPARC64-SAME: "-L[[SYSROOT]]/usr/lib/sparcv9"
+// CHECK-LD-SPARC64-NOT:  "-zignore"
+// CHECK-LD-SPARC64-NOT:  "-latomic"
+// CHECK-LD-SPARC64-NOT:  "-zrecord"
 // CHECK-LD-SPARC64-SAME: "-lgcc_s"
 // CHECK-LD-SPARC64-SAME: "-lc"
 // CHECK-LD-SPARC64-SAME: "-lgcc"
@@ -63,6 +69,9 @@
 // CHECK-LD-X32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4"
 // CHECK-LD-X32-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/../../.."
 // CHECK-LD-X32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-X32-NOT:  "-zignore"
+// CHECK-LD-X32-NOT:  "-latomic"
+// CHECK-LD-X32-NOT:  "-zrecord"
 // CHECK-LD-X32-SAME: "-lgcc_s"
 // CHECK-LD-X32-SAME: "-lc"
 // CHECK-LD-X32-SAME: "-lgcc"
@@ -86,6 +95,9 @@
 // CHECK-LD-X64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/amd64"
 // CHECK-LD-X64-SAME: 
"-L[[SYSROOT]]/usr/gcc/4.9/lib/gcc/i386-pc-solaris2.11/4.9.4/../../../amd64"
 // CHECK-LD-X64-SAME: "-L[[SYSROOT]]/usr/lib/amd64"
+// CHECK-LD-X64-NOT:  "-zignore"
+// CHECK-LD-X64-NOT:  "-latomic"
+// CHECK-LD-X64-NOT:  "-zrecord"
 // CHECK-LD-X64-SAME: "-lgcc_s"
 // CHECK-LD-X64-SAME: "-lc"
 // CHECK-LD-X64-SAME: "-lgcc"
Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+// forcibly link with libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -117,6 +117,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -756,7 +756,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 


Index: clang/test/Driver/solaris-ld.c
===
--- clang/test/Driver/solaris-ld.c
+++ clang/test/Driver/solaris-ld.c
@@ -17,6 +17,9 @@
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2"
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/gcc/4.8/lib/gcc/sparc-sun-solaris2.11/4.8.2/../../.."
 // CHECK-LD-SPARC32-SAME: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-SPARC32-SAME: "-zignore"
+//

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D118021#3307079 , @MaskRay wrote:

> This needs some tests, otherwise there is a risk that others may break your 
> changes when refactoring driver code.

I thought it would be enough that any breakage there would cause a large number 
of test failures.  But it's certainly better to avoid problems up from rather 
than reacting afterwards.

> The style I favor most is `test/Driver/linux-cross.cpp`. freebsd.c is not too 
> bad. linux-ld.c is somewhat messy but can give you some idea what to test.
> I'll add a note that zignore isn't tested and that is a problem.

It seemed easiest to add to `solaris-ld.c`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

This needs some tests, otherwise there is a risk that others may break your 
changes when refactoring driver code.

The style I favor most is `test/Driver/linux-cross.cpp`. freebsd.c is not too 
bad. linux-ld.c is somewhat messy but can give you some idea what to test.
I'll add a note that zignore isn't tested and that is a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

Ping^2.  It would be great if this could be reviewed/commited soon: it fixes 
hundreds of testsuite failures, so I'd like to get it into LLVM 14.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

Ping? It's been a week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

glaubitz wrote:
> glaubitz wrote:
> > ro wrote:
> > > ro wrote:
> > > > glaubitz wrote:
> > > > > ro wrote:
> > > > > > glaubitz wrote:
> > > > > > > joerg wrote:
> > > > > > > > This comment is misleading. It's not so much that LLVM doesn't 
> > > > > > > > support them, but that SPARCv8 doesn't have the necessary 
> > > > > > > > hardware support. The v8+ support is incomplete, which is a 
> > > > > > > > related problem though.
> > > > > > > As far as I know, 64-bit atomics are supported if you enable V8+ 
> > > > > > > in GCC - without linking against `libatomic`:
> > > > > > > 
> > > > > > > ```
> > > > > > > glaubitz@gcc202:~$ cat atomic.c
> > > > > > > #include 
> > > > > > > 
> > > > > > > int main()
> > > > > > > {
> > > > > > >   int64_t x = 0, y = 1;
> > > > > > >   y = __sync_val_compare_and_swap(&x, x, y);
> > > > > > >   return 0;
> > > > > > > }
> > > > > > > glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > > > > > > glaubitz@gcc202:~$
> > > > > > > ```
> > > > > > I know, that's why I referred to my patch to default `clang` on
> > > > > > Solaris/sparc to V8+.  I'll update the comment.
> > > > > > 
> > > > > > I'd tried to actually fix the underlying issue (`clang` not emitting
> > > > > > `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> > > > > > areas of LLVM I know nothing about.  I might post a WIP patch
> > > > > > for reference since there are several issues there.
> > > > > I think @jrtc27 might be able to give advise here having the 
> > > > > knowledge on the Tablegen stuff (if my mind serves me right).
> > > > > 
> > > > > The disassembly shows in any case that `casx` is being emitted as you 
> > > > > say:
> > > > > 
> > > > > ```
> > > > > 69c:   c7 f0 50 02 casx  [ %g1 ], %g2, %g3
> > > > > ```
> > > > Of course it does, thus my reference to `unlike gcc` in the
> > > > summary.  What Joerg meant, I believe, is that V8+ support
> > > > **in LLVM** is incomplete.
> > > Right, `gcc` does all of this correctly.  One LLVM issue, e.g., is
> > > that it handles `casx` as 64-bit only (cf. `SparcInstr64Bit.td`)
> > > while it should be guarded by `HasV9` instead.
> > Interesting. Would `SparcInstr64Bit.td` actually get used when targeting 
> > 32-bit SPARC?
> > 
> > If yes, it looks like replacing the guards `Is64Bit` with `HasV9` should do 
> > the trick, shouldn't it?
> Ah, the header of `SparcInstr64Bit.td` actually has the following comment:
> 
> ```
> // This file contains instruction definitions and patterns needed for 64-bit
> // code generation on SPARC v9.
> //
> // Some SPARC v9 instructions are defined in SparcInstrInfo.td because they 
> can
> // also be used in 32-bit code running on a SPARC v9 CPU.
> ```
> 
> So, I guess move the 64-bit atomics stuff out of `SparcInstr64.td` and guard 
> it with `HasV9` instead of `Is64Bit`?
The hard part is adding support for using 64-bit registers on 32-bit targets, 
in general.  You need to declare the register class correctly, and call 
addRegisterClass() in SparcISelLowering.cpp to declare that 64-bit integer 
registers exist.  Once you mark the 64-bit registers legal, legalization rules 
for a bunch of basic operations will change, so you'll need to do a bunch of 
work to get basic 64-bit arithmetic/load/store/etc. working again.

Once you have all that, telling isel to emit 64-bit atomics should be easy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

glaubitz wrote:
> ro wrote:
> > ro wrote:
> > > glaubitz wrote:
> > > > ro wrote:
> > > > > glaubitz wrote:
> > > > > > joerg wrote:
> > > > > > > This comment is misleading. It's not so much that LLVM doesn't 
> > > > > > > support them, but that SPARCv8 doesn't have the necessary 
> > > > > > > hardware support. The v8+ support is incomplete, which is a 
> > > > > > > related problem though.
> > > > > > As far as I know, 64-bit atomics are supported if you enable V8+ in 
> > > > > > GCC - without linking against `libatomic`:
> > > > > > 
> > > > > > ```
> > > > > > glaubitz@gcc202:~$ cat atomic.c
> > > > > > #include 
> > > > > > 
> > > > > > int main()
> > > > > > {
> > > > > >   int64_t x = 0, y = 1;
> > > > > >   y = __sync_val_compare_and_swap(&x, x, y);
> > > > > >   return 0;
> > > > > > }
> > > > > > glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > > > > > glaubitz@gcc202:~$
> > > > > > ```
> > > > > I know, that's why I referred to my patch to default `clang` on
> > > > > Solaris/sparc to V8+.  I'll update the comment.
> > > > > 
> > > > > I'd tried to actually fix the underlying issue (`clang` not emitting
> > > > > `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> > > > > areas of LLVM I know nothing about.  I might post a WIP patch
> > > > > for reference since there are several issues there.
> > > > I think @jrtc27 might be able to give advise here having the knowledge 
> > > > on the Tablegen stuff (if my mind serves me right).
> > > > 
> > > > The disassembly shows in any case that `casx` is being emitted as you 
> > > > say:
> > > > 
> > > > ```
> > > > 69c:   c7 f0 50 02 casx  [ %g1 ], %g2, %g3
> > > > ```
> > > Of course it does, thus my reference to `unlike gcc` in the
> > > summary.  What Joerg meant, I believe, is that V8+ support
> > > **in LLVM** is incomplete.
> > Right, `gcc` does all of this correctly.  One LLVM issue, e.g., is
> > that it handles `casx` as 64-bit only (cf. `SparcInstr64Bit.td`)
> > while it should be guarded by `HasV9` instead.
> Interesting. Would `SparcInstr64Bit.td` actually get used when targeting 
> 32-bit SPARC?
> 
> If yes, it looks like replacing the guards `Is64Bit` with `HasV9` should do 
> the trick, shouldn't it?
Ah, the header of `SparcInstr64Bit.td` actually has the following comment:

```
// This file contains instruction definitions and patterns needed for 64-bit
// code generation on SPARC v9.
//
// Some SPARC v9 instructions are defined in SparcInstrInfo.td because they can
// also be used in 32-bit code running on a SPARC v9 CPU.
```

So, I guess move the 64-bit atomics stuff out of `SparcInstr64.td` and guard it 
with `HasV9` instead of `Is64Bit`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

ro wrote:
> ro wrote:
> > glaubitz wrote:
> > > ro wrote:
> > > > glaubitz wrote:
> > > > > joerg wrote:
> > > > > > This comment is misleading. It's not so much that LLVM doesn't 
> > > > > > support them, but that SPARCv8 doesn't have the necessary hardware 
> > > > > > support. The v8+ support is incomplete, which is a related problem 
> > > > > > though.
> > > > > As far as I know, 64-bit atomics are supported if you enable V8+ in 
> > > > > GCC - without linking against `libatomic`:
> > > > > 
> > > > > ```
> > > > > glaubitz@gcc202:~$ cat atomic.c
> > > > > #include 
> > > > > 
> > > > > int main()
> > > > > {
> > > > >   int64_t x = 0, y = 1;
> > > > >   y = __sync_val_compare_and_swap(&x, x, y);
> > > > >   return 0;
> > > > > }
> > > > > glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > > > > glaubitz@gcc202:~$
> > > > > ```
> > > > I know, that's why I referred to my patch to default `clang` on
> > > > Solaris/sparc to V8+.  I'll update the comment.
> > > > 
> > > > I'd tried to actually fix the underlying issue (`clang` not emitting
> > > > `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> > > > areas of LLVM I know nothing about.  I might post a WIP patch
> > > > for reference since there are several issues there.
> > > I think @jrtc27 might be able to give advise here having the knowledge on 
> > > the Tablegen stuff (if my mind serves me right).
> > > 
> > > The disassembly shows in any case that `casx` is being emitted as you say:
> > > 
> > > ```
> > > 69c:   c7 f0 50 02 casx  [ %g1 ], %g2, %g3
> > > ```
> > Of course it does, thus my reference to `unlike gcc` in the
> > summary.  What Joerg meant, I believe, is that V8+ support
> > **in LLVM** is incomplete.
> Right, `gcc` does all of this correctly.  One LLVM issue, e.g., is
> that it handles `casx` as 64-bit only (cf. `SparcInstr64Bit.td`)
> while it should be guarded by `HasV9` instead.
Interesting. Would `SparcInstr64Bit.td` actually get used when targeting 32-bit 
SPARC?

If yes, it looks like replacing the guards `Is64Bit` with `HasV9` should do the 
trick, shouldn't it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 403607.
ro marked an inline comment as done.
ro added a comment.

Omit `sanitizer_atomic_clang.h` part, belongs to D118021 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Solaris.cpp


Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+// forcibly link with libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -114,6 +114,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -737,7 +737,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 


Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+// forcibly link with libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -114,6 +114,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -737,7 +737,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 403604.
ro added a comment.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

- Clarify comment.
- Use `__ATOMIC_SEQ_CST`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Solaris.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h


Index: compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
===
--- compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
+++ compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
@@ -74,13 +74,8 @@
 inline bool atomic_compare_exchange_strong(volatile T *a, typename T::Type 
*cmp,
typename T::Type xchg,
memory_order mo) {
-  typedef typename T::Type Type;
-  Type cmpv = *cmp;
-  Type prev;
-  prev = __sync_val_compare_and_swap(&a->val_dont_use, cmpv, xchg);
-  if (prev == cmpv) return true;
-  *cmp = prev;
-  return false;
+  return __atomic_compare_exchange(&a->val_dont_use, cmp, &xchg, false, mo,
+   __ATOMIC_SEQ_CST);
 }
 
 template
Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+// forcibly link with libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -114,6 +114,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -737,7 +737,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 


Index: compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
===
--- compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
+++ compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
@@ -74,13 +74,8 @@
 inline bool atomic_compare_exchange_strong(volatile T *a, typename T::Type *cmp,
typename T::Type xchg,
memory_order mo) {
-  typedef typename T::Type Type;
-  Type cmpv = *cmp;
-  Type prev;
-  prev = __sync_val_compare_and_swap(&a->val_dont_use, cmpv, xchg);
-  if (prev == cmpv) return true;
-  *cmp = prev;
-  return false;
+  return __atomic_compare_exchange(&a->val_dont_use, cmp, &xchg, false, mo,
+   __ATOMIC_SEQ_CST);
 }
 
 template
Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM support for atomics on 32-bit SPARC V8+ is incomplete, so
+// forcibly link with libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:138-139
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));

ro wrote:
> glaubitz wrote:
> > Note, this will only work when `__atomic_compare_exchange()` is being used 
> > as ``__sync_val_compare_and_swap_8` is not implemented by `libatomic` in 
> > gcc, see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63368 (Unless this 
> > has changed recently).
> True, that the summary said `this patch works around the first of those`.  
> The other part is now D118024.
Ah, sorry. I missed that he was specifically talking about SPARCv8.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

glaubitz wrote:
> ro wrote:
> > glaubitz wrote:
> > > joerg wrote:
> > > > This comment is misleading. It's not so much that LLVM doesn't support 
> > > > them, but that SPARCv8 doesn't have the necessary hardware support. The 
> > > > v8+ support is incomplete, which is a related problem though.
> > > As far as I know, 64-bit atomics are supported if you enable V8+ in GCC - 
> > > without linking against `libatomic`:
> > > 
> > > ```
> > > glaubitz@gcc202:~$ cat atomic.c
> > > #include 
> > > 
> > > int main()
> > > {
> > >   int64_t x = 0, y = 1;
> > >   y = __sync_val_compare_and_swap(&x, x, y);
> > >   return 0;
> > > }
> > > glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > > glaubitz@gcc202:~$
> > > ```
> > I know, that's why I referred to my patch to default `clang` on
> > Solaris/sparc to V8+.  I'll update the comment.
> > 
> > I'd tried to actually fix the underlying issue (`clang` not emitting
> > `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> > areas of LLVM I know nothing about.  I might post a WIP patch
> > for reference since there are several issues there.
> I think @jrtc27 might be able to give advise here having the knowledge on the 
> Tablegen stuff (if my mind serves me right).
> 
> The disassembly shows in any case that `casx` is being emitted as you say:
> 
> ```
> 69c:   c7 f0 50 02 casx  [ %g1 ], %g2, %g3
> ```
Of course it does, thus my reference to `unlike gcc` in the
summary.  What Joerg meant, I believe, is that V8+ support
**in LLVM** is incomplete.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

ro wrote:
> glaubitz wrote:
> > ro wrote:
> > > glaubitz wrote:
> > > > joerg wrote:
> > > > > This comment is misleading. It's not so much that LLVM doesn't 
> > > > > support them, but that SPARCv8 doesn't have the necessary hardware 
> > > > > support. The v8+ support is incomplete, which is a related problem 
> > > > > though.
> > > > As far as I know, 64-bit atomics are supported if you enable V8+ in GCC 
> > > > - without linking against `libatomic`:
> > > > 
> > > > ```
> > > > glaubitz@gcc202:~$ cat atomic.c
> > > > #include 
> > > > 
> > > > int main()
> > > > {
> > > >   int64_t x = 0, y = 1;
> > > >   y = __sync_val_compare_and_swap(&x, x, y);
> > > >   return 0;
> > > > }
> > > > glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > > > glaubitz@gcc202:~$
> > > > ```
> > > I know, that's why I referred to my patch to default `clang` on
> > > Solaris/sparc to V8+.  I'll update the comment.
> > > 
> > > I'd tried to actually fix the underlying issue (`clang` not emitting
> > > `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> > > areas of LLVM I know nothing about.  I might post a WIP patch
> > > for reference since there are several issues there.
> > I think @jrtc27 might be able to give advise here having the knowledge on 
> > the Tablegen stuff (if my mind serves me right).
> > 
> > The disassembly shows in any case that `casx` is being emitted as you say:
> > 
> > ```
> > 69c:   c7 f0 50 02 casx  [ %g1 ], %g2, %g3
> > ```
> Of course it does, thus my reference to `unlike gcc` in the
> summary.  What Joerg meant, I believe, is that V8+ support
> **in LLVM** is incomplete.
Right, `gcc` does all of this correctly.  One LLVM issue, e.g., is
that it handles `casx` as 64-bit only (cf. `SparcInstr64Bit.td`)
while it should be guarded by `HasV9` instead.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:138-139
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));

glaubitz wrote:
> Note, this will only work when `__atomic_compare_exchange()` is being used as 
> ``__sync_val_compare_and_swap_8` is not implemented by `libatomic` in gcc, 
> see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63368 (Unless this has 
> changed recently).
True, that the summary said `this patch works around the first of those`.  The 
other part is now D118024.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

ro wrote:
> glaubitz wrote:
> > joerg wrote:
> > > This comment is misleading. It's not so much that LLVM doesn't support 
> > > them, but that SPARCv8 doesn't have the necessary hardware support. The 
> > > v8+ support is incomplete, which is a related problem though.
> > As far as I know, 64-bit atomics are supported if you enable V8+ in GCC - 
> > without linking against `libatomic`:
> > 
> > ```
> > glaubitz@gcc202:~$ cat atomic.c
> > #include 
> > 
> > int main()
> > {
> >   int64_t x = 0, y = 1;
> >   y = __sync_val_compare_and_swap(&x, x, y);
> >   return 0;
> > }
> > glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > glaubitz@gcc202:~$
> > ```
> I know, that's why I referred to my patch to default `clang` on
> Solaris/sparc to V8+.  I'll update the comment.
> 
> I'd tried to actually fix the underlying issue (`clang` not emitting
> `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> areas of LLVM I know nothing about.  I might post a WIP patch
> for reference since there are several issues there.
I think @jrtc27 might be able to give advise here having the knowledge on the 
Tablegen stuff (if my mind serves me right).

The disassembly shows in any case that `casx` is being emitted as you say:

```
69c:   c7 f0 50 02 casx  [ %g1 ], %g2, %g3
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

glaubitz wrote:
> joerg wrote:
> > This comment is misleading. It's not so much that LLVM doesn't support 
> > them, but that SPARCv8 doesn't have the necessary hardware support. The v8+ 
> > support is incomplete, which is a related problem though.
> As far as I know, 64-bit atomics are supported if you enable V8+ in GCC - 
> without linking against `libatomic`:
> 
> ```
> glaubitz@gcc202:~$ cat atomic.c
> #include 
> 
> int main()
> {
>   int64_t x = 0, y = 1;
>   y = __sync_val_compare_and_swap(&x, x, y);
>   return 0;
> }
> glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> glaubitz@gcc202:~$
> ```
I know, that's why I referred to my patch to default `clang` on
Solaris/sparc to V8+.  I'll update the comment.

I'd tried to actually fix the underlying issue (`clang` not emitting
`casx` with `-m32 -mcpu=v9`), but ran into internal errors and
areas of LLVM I know nothing about.  I might post a WIP patch
for reference since there are several issues there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

joerg wrote:
> This comment is misleading. It's not so much that LLVM doesn't support them, 
> but that SPARCv8 doesn't have the necessary hardware support. The v8+ support 
> is incomplete, which is a related problem though.
As far as I know, 64-bit atomics are supported if you enable V8+ in GCC - 
without linking against `libatomic`:

```
glaubitz@gcc202:~$ cat atomic.c
#include 

int main()
{
  int64_t x = 0, y = 1;
  y = __sync_val_compare_and_swap(&x, x, y);
  return 0;
}
glaubitz@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
glaubitz@gcc202:~$
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.

This comment is misleading. It's not so much that LLVM doesn't support them, 
but that SPARCv8 doesn't have the necessary hardware support. The v8+ support 
is incomplete, which is a related problem though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:138-139
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));

Note, this will only work when `__atomic_compare_exchange()` is being used as 
``__sync_val_compare_and_swap_8` is not implemented by `libatomic` in gcc, see: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63368 (Unless this has changed 
recently).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

Ah, I actually ran into this very problem! Thanks for fixing this.

Will test this today on Linux and report back!

Could you maybe have a look at my updated revision here? 
https://reviews.llvm.org/D98575

I think this particular change should be save on Solaris as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021

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

[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[Rainer Orth via Phabricator via cfe-commits]

ro created this revision.
ro added reviewers: jyknight, venkatra, efriedma, jrtc27.
ro added a project: clang.
Herald added a subscriber: fedor.sergeev.
ro requested review of this revision.
Herald added a subscriber: cfe-commits.

Even after D86621 , `clang -m32` on 
Solaris/sparcv9 doesn't inline atomics with 8-byte
operands, unlike `gcc`.  This leads to many link failures in the testsuite 
(undefined references
to `__atomic_load_8` and `__sync_val_compare_and_swap_8`.  Until a proper 
codegen fix can be implemented, this patch works around the first of those by 
linking with `-latomic`.

Tested on `sparcv9-sun-solaris2.11`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118021

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Solaris.cpp


Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -114,6 +114,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -737,7 +737,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 


Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -132,6 +132,13 @@
   CmdArgs.push_back("-lssp_nonshared");
   CmdArgs.push_back("-lssp");
 }
+// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+// libatomic as a workaround.
+if (getToolChain().getTriple().getArch() == llvm::Triple::sparc) {
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), true));
+  CmdArgs.push_back("-latomic");
+  CmdArgs.push_back(getAsNeededOption(getToolChain(), false));
+}
 CmdArgs.push_back("-lgcc_s");
 CmdArgs.push_back("-lc");
 if (!Args.hasArg(options::OPT_shared)) {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -114,6 +114,8 @@
   bool ForceStaticHostRuntime = false,
   bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
+const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
+
 llvm::opt::Arg *getLastProfileUseArg(const llvm::opt::ArgList &Args);
 llvm::opt::Arg *getLastProfileSampleUseArg(const llvm::opt::ArgList &Args);
 
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -737,7 +737,7 @@
   return false;
 }
 
-static const char *getAsNeededOption(const ToolChain &TC, bool as_needed) {
+const char *tools::getAsNeededOption(const ToolChain &TC, bool as_needed) {
   assert(!TC.getTriple().isOSAIX() &&
  "AIX linker does not support any form of --as-needed option yet.");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits