Fix libstdc++ tests requiring atomic support on hppa-hpux

Message ID 54b2ad46-4e9c-1abe-07f9-7d123ff63c47@bell.net
State New
Headers show
Series
  • Fix libstdc++ tests requiring atomic support on hppa-hpux
Related show

Commit Message

John David Anglin March 9, 2019, 5:46 p.m.
The hppa*-*-hpux* target has no builtin atomic support, so we need to explicitly link
applications requiring atomic support against libatomic.

Okay?

Dave
-- 
John David Anglin  dave.anglin@bell.net

2019-03-09  John David Anglin  <dave.anglin@bell.net>

	PR libstdc++/89461
	* testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc: Link
	against libatomic on hppa*-*-hpux*.
	* testsuite/20_util/shared_ptr/thread/mutex_weaktoshared.cc: Likewise.
	* testsuite/experimental/net/timer/waitable/cons.cc: Likewise.
	* testsuite/experimental/net/timer/waitable/dest.cc: Likewise.
	* testsuite/experimental/net/timer/waitable/ops.cc: Likewise.
	* testsuite/lib/libstdc++.exp: Locate libatomic.

Comments

Jonathan Wakely March 11, 2019, 12:01 p.m. | #1
On 09/03/19 12:46 -0500, John David Anglin wrote:
>The hppa*-*-hpux* target has no builtin atomic support, so we need to explicitly link

>applications requiring atomic support against libatomic.

>

>Okay?

>

>Dave

>-- 

>John David Anglin  dave.anglin@bell.net

>

>2019-03-09  John David Anglin  <dave.anglin@bell.net>

>

>	PR libstdc++/89461

>	* testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc: Link

>	against libatomic on hppa*-*-hpux*.

>	* testsuite/20_util/shared_ptr/thread/mutex_weaktoshared.cc: Likewise.

>	* testsuite/experimental/net/timer/waitable/cons.cc: Likewise.

>	* testsuite/experimental/net/timer/waitable/dest.cc: Likewise.

>	* testsuite/experimental/net/timer/waitable/ops.cc: Likewise.

>	* testsuite/lib/libstdc++.exp: Locate libatomic.



What do you think about adding the following?

--- a/libstdc++-v3/testsuite/lib/dg-options.exp
+++ b/libstdc++-v3/testsuite/lib/dg-options.exp
@@ -257,6 +257,15 @@ proc add_options_for_net_ts { flags } {
     return $flags
 }

+# Add to FLAGS all the target-specific flags to link to libatomic, if required.
+
+proc add_options_for_libatomic { flags } {
+    if { [istarget hppa*-*-hpux*] } {
+       return "$flags -L../../libatomic/.libs -latomic"
+    }
+    return $flags
+}
+
 # Like dg-options, but adds to the default options rather than replacing them.

 proc dg-additional-options { args } {



>

>Index: testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc

>===================================================================

>--- testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc	(revision 269442)

>+++ testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc	(working copy)

>@@ -22,6 +22,7 @@

> // { dg-require-effective-target c++11 }

> // { dg-require-effective-target pthread }

> // { dg-require-cstdint "" }

>+// { dg-additional-options "-L../../libatomic/.libs -latomic" { target hppa*-*-hpux* } }


And then this would be:

--- a/libstdc++-v3/testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc
+++ b/libstdc++-v3/testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc
@@ -19,6 +19,7 @@

 // { dg-do run }
 // { dg-options "-pthread"  }
+// { dg-add-options libatomic }
 // { dg-require-effective-target c++11 }
 // { dg-require-effective-target pthread }
 // { dg-require-cstdint "" }
Andreas Schwab March 11, 2019, 1:13 p.m. | #2
On Mär 11 2019, Jonathan Wakely <jwakely@redhat.com> wrote:

> What do you think about adding the following?

>

> --- a/libstdc++-v3/testsuite/lib/dg-options.exp

> +++ b/libstdc++-v3/testsuite/lib/dg-options.exp

> @@ -257,6 +257,15 @@ proc add_options_for_net_ts { flags } {

>     return $flags

> }

>

> +# Add to FLAGS all the target-specific flags to link to libatomic, if required.

> +

> +proc add_options_for_libatomic { flags } {

> +    if { [istarget hppa*-*-hpux*] } {

> +       return "$flags -L../../libatomic/.libs -latomic"

> +    }

> +    return $flags

> +}


That should probably also be enabled for riscv*-*-*.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Jonathan Wakely March 11, 2019, 1:16 p.m. | #3
On 11/03/19 14:13 +0100, Andreas Schwab wrote:
>On Mär 11 2019, Jonathan Wakely <jwakely@redhat.com> wrote:

>

>> What do you think about adding the following?

>>

>> --- a/libstdc++-v3/testsuite/lib/dg-options.exp

>> +++ b/libstdc++-v3/testsuite/lib/dg-options.exp

>> @@ -257,6 +257,15 @@ proc add_options_for_net_ts { flags } {

>>     return $flags

>> }

>>

>> +# Add to FLAGS all the target-specific flags to link to libatomic, if required.

>> +

>> +proc add_options_for_libatomic { flags } {

>> +    if { [istarget hppa*-*-hpux*] } {

>> +       return "$flags -L../../libatomic/.libs -latomic"

>> +    }

>> +    return $flags

>> +}

>

>That should probably also be enabled for riscv*-*-*.


So all the more reason to do it this way, so the list of targets is
only maintained in one place.
John David Anglin March 11, 2019, 1:27 p.m. | #4
On 2019-03-11 9:16 a.m., Jonathan Wakely wrote:
> On 11/03/19 14:13 +0100, Andreas Schwab wrote:

>> On Mär 11 2019, Jonathan Wakely <jwakely@redhat.com> wrote:

>>

>>> What do you think about adding the following?

>>>

>>> --- a/libstdc++-v3/testsuite/lib/dg-options.exp

>>> +++ b/libstdc++-v3/testsuite/lib/dg-options.exp

>>> @@ -257,6 +257,15 @@ proc add_options_for_net_ts { flags } {

>>>     return $flags

>>> }

>>>

>>> +# Add to FLAGS all the target-specific flags to link to libatomic, if required.

>>> +

>>> +proc add_options_for_libatomic { flags } {

>>> +    if { [istarget hppa*-*-hpux*] } {

>>> +       return "$flags -L../../libatomic/.libs -latomic"

>>> +    }

>>> +    return $flags

>>> +}

>>

>> That should probably also be enabled for riscv*-*-*.

>

> So all the more reason to do it this way, so the list of targets is

> only maintained in one place

Your suggestion looks good to me.  We still need hunk that sets up LD_LIBRARY_PATH.

This is what is done in fortran tests:
! { dg-additional-options "-latomic" { target libatomic_available } }

Dave

-- 
John David Anglin  dave.anglin@bell.net
Jonathan Wakely March 11, 2019, 1:54 p.m. | #5
On 11/03/19 09:27 -0400, John David Anglin wrote:
>On 2019-03-11 9:16 a.m., Jonathan Wakely wrote:

>> On 11/03/19 14:13 +0100, Andreas Schwab wrote:

>>> On Mär 11 2019, Jonathan Wakely <jwakely@redhat.com> wrote:

>>>

>>>> What do you think about adding the following?

>>>>

>>>> --- a/libstdc++-v3/testsuite/lib/dg-options.exp

>>>> +++ b/libstdc++-v3/testsuite/lib/dg-options.exp

>>>> @@ -257,6 +257,15 @@ proc add_options_for_net_ts { flags } {

>>>>     return $flags

>>>> }

>>>>

>>>> +# Add to FLAGS all the target-specific flags to link to libatomic, if required.

>>>> +

>>>> +proc add_options_for_libatomic { flags } {

>>>> +    if { [istarget hppa*-*-hpux*] } {

>>>> +       return "$flags -L../../libatomic/.libs -latomic"

>>>> +    }

>>>> +    return $flags

>>>> +}

>>>

>>> That should probably also be enabled for riscv*-*-*.

>>

>> So all the more reason to do it this way, so the list of targets is

>> only maintained in one place

>Your suggestion looks good to me.  We still need hunk that sets up LD_LIBRARY_PATH.


Right.

>This is what is done in fortran tests:

>! { dg-additional-options "-latomic" { target libatomic_available } }


That adds it for every target that has a libatomic though, right? We
have tests that check certain uses of atomics work without libatomic,
which would no longer check anything if we added that to them :-)

If we use { dg-add-options libatomic } it'll only be added for the
listed targets (hpux and riscv to begin with) and if somebody disables
building libatomic on those targets they probably should get test
failures to say there's something wrong.
Jonathan Wakely March 11, 2019, 2:01 p.m. | #6
On 11/03/19 13:54 +0000, Jonathan Wakely wrote:
>On 11/03/19 09:27 -0400, John David Anglin wrote:

>>On 2019-03-11 9:16 a.m., Jonathan Wakely wrote:

>>>On 11/03/19 14:13 +0100, Andreas Schwab wrote:

>>>>On Mär 11 2019, Jonathan Wakely <jwakely@redhat.com> wrote:

>>>>

>>>>>What do you think about adding the following?

>>>>>

>>>>>--- a/libstdc++-v3/testsuite/lib/dg-options.exp

>>>>>+++ b/libstdc++-v3/testsuite/lib/dg-options.exp

>>>>>@@ -257,6 +257,15 @@ proc add_options_for_net_ts { flags } {

>>>>>    return $flags

>>>>>}

>>>>>

>>>>>+# Add to FLAGS all the target-specific flags to link to libatomic, if required.

>>>>>+

>>>>>+proc add_options_for_libatomic { flags } {

>>>>>+    if { [istarget hppa*-*-hpux*] } {

>>>>>+       return "$flags -L../../libatomic/.libs -latomic"

>>>>>+    }

>>>>>+    return $flags

>>>>>+}

>>>>

>>>>That should probably also be enabled for riscv*-*-*.

>>>

>>>So all the more reason to do it this way, so the list of targets is

>>>only maintained in one place

>>Your suggestion looks good to me.  We still need hunk that sets up LD_LIBRARY_PATH.

>

>Right.

>

>>This is what is done in fortran tests:

>>! { dg-additional-options "-latomic" { target libatomic_available } }

>

>That adds it for every target that has a libatomic though, right? We

>have tests that check certain uses of atomics work without libatomic,

>which would no longer check anything if we added that to them :-)


Ah, but unless -L../../libatomic./libs has been added to the flags,
-latomic won't find anything, so it wouldn't get added.

I still think {target libatomic_available } doesn't buy us much, due
to ...

>If we use { dg-add-options libatomic } it'll only be added for the

>listed targets (hpux and riscv to begin with) and if somebody disables

>building libatomic on those targets they probably should get test

>failures to say there's something wrong.

>
Joseph Myers March 12, 2019, 11:01 p.m. | #7
On Mon, 11 Mar 2019, Jonathan Wakely wrote:

> +proc add_options_for_libatomic { flags } {

> +    if { [istarget hppa*-*-hpux*] } {

> +       return "$flags -L../../libatomic/.libs -latomic"

> +    }


It's generally inappropriate to hardcode such ../../libatomic/.libs paths 
without making sure it's for build-tree rather than installed testing (for 
installed testing, -latomic can be found without any -L option and it's 
the board file's responsibility to set LD_LIBRARY_PATH if necessary).

-- 
Joseph S. Myers
joseph@codesourcery.com
Jonathan Wakely March 12, 2019, 11:05 p.m. | #8
On 12/03/19 23:01 +0000, Joseph Myers wrote:
>On Mon, 11 Mar 2019, Jonathan Wakely wrote:

>

>> +proc add_options_for_libatomic { flags } {

>> +    if { [istarget hppa*-*-hpux*] } {

>> +       return "$flags -L../../libatomic/.libs -latomic"

>> +    }

>

>It's generally inappropriate to hardcode such ../../libatomic/.libs paths

>without making sure it's for build-tree rather than installed testing (for

>installed testing, -latomic can be found without any -L option and it's

>the board file's responsibility to set LD_LIBRARY_PATH if necessary).


I was going to say something about that, but then saw we already do it
for libgomp/.libs as well. Two wrongs now though ...
John David Anglin March 12, 2019, 11:35 p.m. | #9
On 2019-03-12 7:05 p.m., Jonathan Wakely wrote:
> On 12/03/19 23:01 +0000, Joseph Myers wrote:

>> On Mon, 11 Mar 2019, Jonathan Wakely wrote:

>>

>>> +proc add_options_for_libatomic { flags } {

>>> +    if { [istarget hppa*-*-hpux*] } {

>>> +       return "$flags -L../../libatomic/.libs -latomic"

>>> +    } option.

>>

>> It's generally inappropriate to hardcode such ../../libatomic/.libs paths

>> without making sure it's for build-tree rather than installed testing (for

>> installed testing, -latomic can be found without any -L option and it's

>> the board file's responsibility to set LD_LIBRARY_PATH if necessary).

>

> I was going to say something about that, but then saw we already do it

> for libgomp/.libs as well. Two wrongs now though ..

I just installed the change before I saw Joseph's comment.  I don't believe -latomic can be
found without a -L option in the libstdc++ testsuite.  That's why it's done for libgomp.

However, it's not needed for c, c++, fortran testsuites.  They also setup LD_LIBRARY_PATH
for testing in build tree.  I've never had to use a board file to get this right.

-- 
John David Anglin  dave.anglin@bell.net
Hans-Peter Nilsson March 13, 2019, 1:50 a.m. | #10
Regarding this sometimes-add--latomic(-in-testsuite) that is
revisited:

When is it appropriate to make the user add -latomic to link
their program?  Perhaps different answers for fortran and C++.
I'm guessing "always when using any atomic construct" for C.

I had a grep-look in gcc/doc before asking; I can't see we say
anything.

I don't think "when the target lacks certain atomic features" is
a full valid answer, except pragmatically, to paper over a bug.

brgds, H-P
PS. I'll do the patch if you (someone) does the words.
Joseph Myers March 13, 2019, 1:57 a.m. | #11
On Tue, 12 Mar 2019, Hans-Peter Nilsson wrote:

> When is it appropriate to make the user add -latomic to link

> their program?  Perhaps different answers for fortran and C++.

> I'm guessing "always when using any atomic construct" for C.


I think we should link with --as-needed -latomic --no-as-needed by default 
when --as-needed is supported (see bug 81358).

-- 
Joseph S. Myers
joseph@codesourcery.com

Patch

Index: testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc
===================================================================
--- testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc	(revision 269442)
+++ testsuite/20_util/shared_ptr/thread/default_weaktoshared.cc	(working copy)
@@ -22,6 +22,7 @@ 
 // { dg-require-effective-target c++11 }
 // { dg-require-effective-target pthread }
 // { dg-require-cstdint "" }
+// { dg-additional-options "-L../../libatomic/.libs -latomic" { target hppa*-*-hpux* } }

 #include <memory>
 #include <random>
Index: testsuite/20_util/shared_ptr/thread/mutex_weaktoshared.cc
===================================================================
--- testsuite/20_util/shared_ptr/thread/mutex_weaktoshared.cc	(revision 269442)
+++ testsuite/20_util/shared_ptr/thread/mutex_weaktoshared.cc	(working copy)
@@ -22,6 +22,7 @@ 
 // { dg-require-effective-target c++11 }
 // { dg-require-effective-target pthread }
 // { dg-require-cstdint "" }
+// { dg-additional-options "-L../../libatomic/.libs -latomic" { target hppa*-*-hpux* } }

 #include <memory>
 #include <random>
Index: testsuite/experimental/net/timer/waitable/cons.cc
===================================================================
--- testsuite/experimental/net/timer/waitable/cons.cc	(revision 269442)
+++ testsuite/experimental/net/timer/waitable/cons.cc	(working copy)
@@ -16,6 +16,7 @@ 
 // <http://www.gnu.org/licenses/>.

 // { dg-do run { target c++14 } }
+// { dg-additional-options "-L../../libatomic/.libs -latomic" { target hppa*-*-hpux* } }

 #include <experimental/timer>
 #include <testsuite_hooks.h>
Index: testsuite/experimental/net/timer/waitable/dest.cc
===================================================================
--- testsuite/experimental/net/timer/waitable/dest.cc	(revision 269442)
+++ testsuite/experimental/net/timer/waitable/dest.cc	(working copy)
@@ -16,6 +16,7 @@ 
 // <http://www.gnu.org/licenses/>.

 // { dg-do run { target c++14 } }
+// { dg-additional-options "-L../../libatomic/.libs -latomic" { target hppa*-*-hpux* } }

 #include <experimental/timer>
 #include <testsuite_hooks.h>
Index: testsuite/experimental/net/timer/waitable/ops.cc
===================================================================
--- testsuite/experimental/net/timer/waitable/ops.cc	(revision 269442)
+++ testsuite/experimental/net/timer/waitable/ops.cc	(working copy)
@@ -16,6 +16,7 @@ 
 // <http://www.gnu.org/licenses/>.

 // { dg-do run { target c++14 } }
+// { dg-additional-options "-L../../libatomic/.libs -latomic" { target hppa*-*-hpux* } }

 #include <experimental/timer>
 #include <testsuite_hooks.h>
Index: testsuite/lib/libstdc++.exp
===================================================================
--- testsuite/lib/libstdc++.exp	(revision 269442)
+++ testsuite/lib/libstdc++.exp	(working copy)
@@ -161,6 +161,17 @@ 
     }
     v3track gccdir 3

+    # Locate libatomic.
+    set v3-libatomic 0
+    set libatomicdir [lookfor_file $blddir/../libatomic .libs/libatomic.$shlib_ext]
+    if {$libatomicdir != ""} {
+	set v3-libatomic 1
+	set libatomicdir [file dirname $libatomicdir]
+	append ld_library_path_tmp ":${libatomicdir}"
+	verbose -log "libatomic support detected"
+    }
+    v3track libatomicdir 3
+
     # Locate libgomp. This is only required for parallel mode.
     set v3-libgomp 0
     set libgompdir [lookfor_file $blddir/../libgomp .libs/libgomp.$shlib_ext]