Message ID | c800ff14-3b8c-d302-acdd-7a6d7fa0174b@codesourcery.com |
---|---|
State | New |
Headers | show |
Series | [libgomp,testsuite] PR testsuite/91884 Add -lquadmath if available | expand |
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
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 { + append lang_link_flags " -lquadmath" + } } append ld_library_path [gcc-set-multilib-library-path $GCC_UNDER_TEST] set_ld_library_path_env_vars diff --git a/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp b/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp index af25a22a522..393518d53f9 100644 --- a/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp +++ b/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp @@ -56,11 +56,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 { + append lang_link_flags " -lquadmath" + } } append ld_library_path [gcc-set-multilib-library-path $GCC_UNDER_TEST] set_ld_library_path_env_vars