[libgomp,testsuite] PR testsuite/91884 Add -lquadmath if available

The test cases in GCC's libraries run via DejaGNU's "[find_gcc]" – 
unless the compiler has been explicitly provided. [1] That means that by 
default everything runs with "<buildir>/gcc/xgcc" – at least for an 
in-build-dir test run.

As that's the C driver, specific libraries such as "-lgfortran" or 
"-lstdc++" have to added explicitly.

The problem with Fortran is: gfortran is built with libquadmath support 
on several platforms – in that case, libgfortran depends on libquadmath. 
But libquadmath support can be disabled or is not present at all on a 
given platform. – With the "gfortran" driver, that's solved by using a 
.spec file. But for xgcc …

For build-tree test runs, it's simple: As we check for the libquadmath 
libraries explicitly, we know an .a or .so file exists and can add 
"-lquadmath" – but for out-of-tree runs, we cannot. – Hence, I propose a 
link test.

What do you think of the attached patch? (Lightly tested on x86_64.)


Background (see also PR):

On x86_64 the issue is not visible. As Joseph pointed out internally 
(half a year ago): The ELF linker has a (mis)feature where it 
automatically looks for libraries named in DT_NEEDED, emulating how the 
dynamic linker searches and using directories specified in -rpath-link 
if necessary. See ld/emultempl/elf32.em.

But on systems like PowerPC, the linker explicitly requires "-lquadmath" 
in order that the linking succeeds.

Tobias

[1] By contrast, those tests under gcc/testsuite/ call functions like 
"gfortran_init" of "./lib/gfortran.exp" instead to get the proper 
driver  (gfortran, g++ etc.). But this function assume that the compiler 
is relative to current working directly (base_dir) at 
$base_dir/../../<name>. That's the case for the tests under 
gcc/testsuite but not for the testsuite in the libraries.

PS: In principle, the same issue occurs with offloading – and has to be 
fixed there as well. However, nvptx and AMD GCN unsurprisingly don't 
have a libquadmath; though, Intel MIC might …

On Tue, Oct 08, 2019 at 08:33:03PM +0200, Tobias Burnus wrote:
> 	libgomp/
> 	* testsuite/libgomp.fortran/fortran.exp: Add -lquadmath if available.
> 	* testsuite/libgomp.oacc-fortran/fortran.exp: Ditto.
> 
> diff --git a/libgomp/testsuite/libgomp.fortran/fortran.exp b/libgomp/testsuite/libgomp.fortran/fortran.exp
> index d848ed4d47f..caffbfe0346 100644
> --- a/libgomp/testsuite/libgomp.fortran/fortran.exp
> +++ b/libgomp/testsuite/libgomp.fortran/fortran.exp
> @@ -54,11 +54,15 @@ if { $lang_test_file_found } {
>  	    # Allow for spec subsitution.
>  	    lappend ALWAYS_CFLAGS "additional_flags=-B${blddir}/${quadmath_library_path}/"
>  	    set ld_library_path "$always_ld_library_path:${blddir}/${lang_library_path}:${blddir}/${quadmath_library_path}"
> +	    append lang_link_flags " -lquadmath"
>  	} else {
>  	    set ld_library_path "$always_ld_library_path:${blddir}/${lang_library_path}"
>  	}
>      } else {
>          set ld_library_path "$always_ld_library_path"
> +        if { [check_no_compiler_messages has_libquadmath executable {int main() {return 0;}} "-lgfortran -lquadmath"] } then {

Can you please split this line?  I know the above lines are quite long too,
but they at least have long strings that aren't easy to split.
While the check_no_compiler_messages are split very commonly in
target-supports.exp, like:
    return [check_no_compiler_messages libatomic_available executable {
        int main (void) { return 0; }
    } "-latomic"]

> +            append lang_link_flags " -lquadmath"
>      } else {
>          set ld_library_path "$always_ld_library_path"
> +        if { [check_no_compiler_messages has_libquadmath executable {int main() {return 0;}} "-lgfortran -lquadmath"] } then {
> +            append lang_link_flags " -lquadmath"
> +        }

Ditto.  Ok with that change.

	Jakub

I have also an alternative version:

* For in-build-dir test runs, do as with previous patch, i.e. add 
"-lquadmath" to the "xgcc" call if the directory is available.

* For installed-compiler test runs, the new patch uses 
GFORTRAN_UNDER_TEST / GXX_UNDER_TEST
(It would also use them with in-build-dir runs, but nothing sets them. I 
have kept the -lgfortran / -lstdg++ flags, though they could be left out 
iff {GXX,GFORTRAN}_UNDER_TEST is set. Presumably, some linker will 
complain if they appear twice.)

Additionally, that patch does some cleanup:
* Only set always_ld_library_path to a blddir path if it exists
* Replace lang_test_file by (existing) lang_test_file_found in lib/*exp

That patch is based on a subset of patches posted by Thomas, extended to 
the new files; cf. PR or:

https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01374.html
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00157.html


What do you think? Which variant do you prefer? If the other one, should 
some cleanup of this patch be used nonetheless?

Tobias

PS: Tested with an in-build-tree test run and an installed-compiler test 
run on x86_64_gnu-linux

Thanks for the review. I have now committed this version as r276754 
(with splitting the long line).

If someone prefers the other variant or bits of it, I will do it as 
follow up.

Cheers,

Tobias

On 10/9/19 9:43 AM, Jakub Jelinek wrote:
> On Tue, Oct 08, 2019 at 08:33:03PM +0200, Tobias Burnus wrote:
>> 	libgomp/
>> 	* testsuite/libgomp.fortran/fortran.exp: Add -lquadmath if available.
>> 	* testsuite/libgomp.oacc-fortran/fortran.exp: Ditto.
>>
>> diff --git a/libgomp/testsuite/libgomp.fortran/fortran.exp b/libgomp/testsuite/libgomp.fortran/fortran.exp
>> index d848ed4d47f..caffbfe0346 100644
>> --- a/libgomp/testsuite/libgomp.fortran/fortran.exp
>> +++ b/libgomp/testsuite/libgomp.fortran/fortran.exp
>> @@ -54,11 +54,15 @@ if { $lang_test_file_found } {
>>   	    # Allow for spec subsitution.
>>   	    lappend ALWAYS_CFLAGS "additional_flags=-B${blddir}/${quadmath_library_path}/"
>>   	    set ld_library_path "$always_ld_library_path:${blddir}/${lang_library_path}:${blddir}/${quadmath_library_path}"
>> +	    append lang_link_flags " -lquadmath"
>>   	} else {
>>   	    set ld_library_path "$always_ld_library_path:${blddir}/${lang_library_path}"
>>   	}
>>       } else {
>>           set ld_library_path "$always_ld_library_path"
>> +        if { [check_no_compiler_messages has_libquadmath executable {int main() {return 0;}} "-lgfortran -lquadmath"] } then {
> Can you please split this line?  I know the above lines are quite long too,
> but they at least have long strings that aren't easy to split.
> While the check_no_compiler_messages are split very commonly in
> target-supports.exp, like:
>      return [check_no_compiler_messages libatomic_available executable {
>          int main (void) { return 0; }
>      } "-latomic"]
>
>> +            append lang_link_flags " -lquadmath"
>>       } else {
>>           set ld_library_path "$always_ld_library_path"
>> +        if { [check_no_compiler_messages has_libquadmath executable {int main() {return 0;}} "-lgfortran -lquadmath"] } then {
>> +            append lang_link_flags " -lquadmath"
>> +        }
> Ditto.  Ok with that change.
>
> 	Jakub

Hi Tobias,

> I have also an alternative version:
>
> * For in-build-dir test runs, do as with previous patch, i.e. add
> "-lquadmath" to the "xgcc" call if the directory is available.
>
> * For installed-compiler test runs, the new patch uses GFORTRAN_UNDER_TEST
> / GXX_UNDER_TEST
> (It would also use them with in-build-dir runs, but nothing sets them. I
> have kept the -lgfortran / -lstdg++ flags, though they could be left out
> iff {GXX,GFORTRAN}_UNDER_TEST is set. Presumably, some linker will complain
> if they appear twice.)

TBH, this whole dance of always using xgcc/gcc and repeating everything
the real language drivers (xg++/g++ resp. gfortran) and their spec files
do and know in the testsuite seems extremly fragile.  The only variant
that makes sense to me is one letting the correct drivers do their work
(as is done e.g. in single-language lib testing like libstdc++) and have
the testsuite provide the necessary -L and -B flags for in-tree builds.

While this how I believe things should work, I've been practically
unable to do any useful testsuite patch review let alone testsuite work
for quite some time, so take this with a large grain of salt...  The
testsuite as a whole is riddled with immense amounts of duplication and
diverging almost-copies of code.

	Rainer