[v2] libgomp/test: Add flags to find libatomic in build-tree testing
diff mbox series

Message ID alpine.LFD.2.21.1911181735050.13542@redsun52.ssa.fujisawa.hgst.com
State Superseded
Headers show
Series
  • [v2] libgomp/test: Add flags to find libatomic in build-tree testing
Related show

Commit Message

Maciej W. Rozycki Nov. 19, 2019, 11:11 p.m. UTC
Add flags to find libatomic in build-tree testing, fixing a catastrophic 
libgomp testsuite failure with targets such as `riscv-linux-gnu' that 
imply `-latomic' with the `-pthread' GCC option, implied in turn by the 
`-fopenacc' and `-fopenmp' options, removing failures like:

.../bin/riscv64-linux-gnu-ld: cannot find -latomic
collect2: error: ld returned 1 exit status
compiler exited with status 1
FAIL: libgomp.c/../libgomp.c-c++-common/atomic-18.c (test for excess errors)
Excess errors:
.../bin/riscv64-linux-gnu-ld: cannot find -latomic

UNRESOLVED: libgomp.c/../libgomp.c-c++-common/atomic-18.c compilation failed to produce executable

and bringing overall test results for the said target (here with the 
`x86_64-linux-gnu' host and RISC-V QEMU in the Linux user emulation mode 
as the target board) from:

		=== libgomp Summary ===

# of expected passes		90
# of unexpected failures	3267
# of expected failures		2
# of unresolved testcases	3247
# of unsupported tests		548

to:

		=== libgomp Summary ===

# of expected passes		6834
# of unexpected failures	4
# of expected failures		4
# of unsupported tests		518

	libgomp/
	* testsuite/lib/libgomp.exp (libgomp_init): Add flags to find 
	libatomic in build-tree testing.
---
Hi Jakub,

 Thank you for your review.

On Mon, 18 Nov 2019, Jakub Jelinek wrote:

> > --- gcc.orig/libgomp/testsuite/lib/libgomp.exp
> > +++ gcc/libgomp/testsuite/lib/libgomp.exp
> > @@ -174,6 +174,16 @@ proc libgomp_init { args } {
> >      # For build-tree testing, also consider the library paths used for builing.
> >      # For installed testing, we assume all that to be provided in the sysroot.
> >      if { $blddir != "" } {
> > +	# Offload options imply `-pthread', and that implies `-latomic'
> > +	# on some targets, so wire in libatomic build directories.
> 
> -fopenmp is not an option I'd like to call Offload option, OpenMP is 
> much more than just offloading, and this isn't on some targets, but only 
> one, riscv*.
> So, I think it should be done only for that target and talk about 
> -fopenmp/-fopenacc options instead of Offload options.

 Thanks for straightening me out on OpenMP; I have adjusted the comment 
and the change description accordingly.

 I think it makes no sense to require people to update this test harness 
if a future target also requires access to libatomic; the `-L' option and 
the dynamic loader path are harmless if not required, as libatomic won't 
actually be pulled unless otherwise requested and there is no other 
library in libatomic's build directory that could be used unintentionally.  
Whereas in the absence of these settings a random version of libatomic 
already present somewhere on the system may accidentally be used and 
affect test results somehow.

 Why do you think it is important that it is only the relevant (i.e. 
`riscv*') targets that the directory providing newly-built libatomic is 
pointed at ?

  Maciej

Changes from v1:

- Replaced references to "offload options" with "`-fopenacc' and 
  `-fopenmp' options".
---
 libgomp/testsuite/lib/libgomp.exp |   11 +++++++++++
 1 file changed, 11 insertions(+)

gcc-test-libgomp-atomic-lib-path.diff

Comments

Jakub Jelinek Nov. 19, 2019, 11:24 p.m. UTC | #1
On Tue, Nov 19, 2019 at 11:11:39PM +0000, Maciej W. Rozycki wrote:
>  Why do you think it is important that it is only the relevant (i.e. 
> `riscv*') targets that the directory providing newly-built libatomic is 
> pointed at ?

Such changes don't happen every day, no other port added it similarly to
riscv during the 2.5 years since riscv had it, and no port needed it ever
before.  With unneeded -L options the log file is larger, one needs to
cut'n'paste more when trying to reproduce stuff etc.

	Jakub

Patch
diff mbox series

Index: gcc/libgomp/testsuite/lib/libgomp.exp
===================================================================
--- gcc.orig/libgomp/testsuite/lib/libgomp.exp
+++ gcc/libgomp/testsuite/lib/libgomp.exp
@@ -174,6 +174,17 @@  proc libgomp_init { args } {
     # For build-tree testing, also consider the library paths used for builing.
     # For installed testing, we assume all that to be provided in the sysroot.
     if { $blddir != "" } {
+	# The `-fopenacc' and `-fopenmp' options imply `-pthread', and
+	# that implies `-latomic' on some targets, so wire in libatomic
+	# build directories.
+	set shlib_ext [get_shlib_extension]
+	set atomic_library_path "${blddir}/../libatomic/.libs"
+	if { [file exists "${atomic_library_path}/libatomic.a"]
+	     || [file exists \
+		 "${atomic_library_path}/libatomic.${shlib_ext}"] } {
+	    lappend ALWAYS_CFLAGS "additional_flags=-L${atomic_library_path}"
+	    append always_ld_library_path ":${atomic_library_path}"
+	}
 	global cuda_driver_include
 	global cuda_driver_lib
 	if { $cuda_driver_include != "" } {