This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC patch: Add libquadmath - and use it in gfortran (round THREE)


Hello,

* Tobias Burnus wrote on Tue, Nov 09, 2010 at 06:20:16PM CET:
> this patch is an updated version of the one posted at
> http://gcc.gnu.org/ml/gcc-patches/2010-11/msg00790.html

> TODO: Fix using gfortran in libgomp. Currently, no .mod files are generated.
> It works if one manually runs "make omp_lib.mod" but it won't happen

I hope this is the most recent version of the patch I'm looking at.
This is a rough review of the build bits, I haven't actually tested
the code.

> --- a/Makefile.def
> +++ b/Makefile.def
> @@ -155,6 +155,7 @@ target_modules = { module= libmudflap; lib_path=.libs; };
>  target_modules = { module= libssp; lib_path=.libs; };
>  target_modules = { module= newlib; };
>  target_modules = { module= libgcc; bootstrap=true; no_check=true; };
> +target_modules = { module= libquadmath; };
>  target_modules = { module= libgfortran; };
>  target_modules = { module= libobjc; };
>  target_modules = { module= libtermcap; no_check=true;
> @@ -571,11 +572,13 @@ dependencies = { module=configure-target-libiberty; on=all-binutils; };
>  dependencies = { module=configure-target-libiberty; on=all-ld; };
>  dependencies = { module=configure-target-newlib; on=all-binutils; };
>  dependencies = { module=configure-target-newlib; on=all-ld; };
> +dependencies = { module=configure-target-libgfortran; on=all-target-libquadmath; };
>  
>  languages = { language=c;	gcc-check-target=check-gcc; };
>  languages = { language=c++;	gcc-check-target=check-c++;
>  				lib-check-target=check-target-libstdc++-v3; };
>  languages = { language=fortran;	gcc-check-target=check-fortran;
> +				lib-check-target=check-target-libquadmath;
>  				lib-check-target=check-target-libgfortran; };

This won't work; the current machinery in Makefile.tpl recognizes only
the first lib-check-target entry, thus will now ignore
check-target-libgfortran.  What you can do however is change in
Makefile.tpl the line

check-[+language+]: check-gcc-[+language+][+ IF lib-check-target +] [+ lib-check-target +][+ ENDIF lib-check-target +]

into:

check-[+language+]: check-gcc-[+language+][+ FOR lib-check-target +] [+ lib-check-target +][+ ENDFOR lib-check-target +]


>  languages = { language=java;	gcc-check-target=check-java;
>  				lib-check-target=check-target-libjava; };

> --- a/gcc/testsuite/lib/gcc-defs.exp
> +++ b/gcc/testsuite/lib/gcc-defs.exp
> @@ -251,8 +251,9 @@ proc gcc-set-multilib-library-path { compiler } {
>  
>      set libpath ":${rootme}"
>      set compiler [lindex $compiler 0]
> +    set options [lrange $compiler 1 end]

Erm, I'm still not tcl-fluent, but wouldn't this line need to come
_before_ the previous one, so $compiler isn't actually overwritten yet?

>      if { [is_remote host] == 0 && [which $compiler] != 0 } {
> -	foreach i "[exec $compiler --print-multi-lib]" {
> +	foreach i "[exec $compiler $options --print-multi-lib]" {
>  	    set mldir ""
>  	    regexp -- "\[a-z0-9=_/\.-\]*;" $i mldir
>  	    set mldir [string trimright $mldir "\;@"]

> --- a/gcc/testsuite/lib/gfortran.exp
> +++ b/gcc/testsuite/lib/gfortran.exp
> @@ -103,11 +103,23 @@ proc gfortran_link_flags { paths } {
>        if [file exists "${gccpath}/libgfortran/libgforbegin.a"] {
>            append flags "-L${gccpath}/libgfortran "
>        }
> +      if [file exists "${gccpath}/libquadmath/.libs/libquadmath.a"] {
> +          # Some targets use libquadmath.a%s in their specs, so they need a -B option
> +          # for uninstalled testing.
> +          append flags "-B${gccpath}/libquadmath/.libs "
> +          append flags "-L${gccpath}/libquadmath/.libs "
> +          append ld_library_path ":${gccpath}/libquadmath/.libs"
> +      }
> +      if [file exists "${gccpath}/libquadmath/.libs/libquadmath.${shlib_ext}"] {
> +	  append flags "-L${gccpath}/libquadmath/.libs "
> +	  append ld_library_path ":${gccpath}/libquadmath/.libs"
> +      }
>        if [file exists "${gccpath}/libiberty/libiberty.a"] {
>            append flags "-L${gccpath}/libiberty "
>        }
> +      # Add ${gccpath}/libgfortran to make sure that the libgfortran.spec file is found
>        append ld_library_path \
> -	[gcc-set-multilib-library-path $GFORTRAN_UNDER_TEST]
> +	[gcc-set-multilib-library-path { ${GFORTRAN_UNDER_TEST} -L${gccpath}/libgfortran } ]
>      }
>  
>      set_ld_library_path_env_vars

> --- a/libgfortran/Makefile.am
> +++ b/libgfortran/Makefile.am
> @@ -34,9 +34,10 @@ LTLDFLAGS = $(shell $(SHELL) $(top_srcdir)/../libtool-ldflags $(LDFLAGS)) \
>  	    -no-undefined -bindir "$(bindir)"
>  
>  toolexeclib_LTLIBRARIES = libgfortran.la
> +toolexeclib_DATA = libgfortran.spec
>  libgfortran_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS)
> -libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` $(LTLDFLAGS) -lm $(extra_ldflags_libgfortran) $(version_arg)
> -libgfortran_la_DEPENDENCIES = $(version_dep)
> +libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' $(srcdir)/libtool-version` $(LTLDFLAGS) $(LIBQUADLIB) -lm $(extra_ldflags_libgfortran) $(version_arg)
> +libgfortran_la_DEPENDENCIES = $(version_dep) libgfortran.spec

Not a dependency on a $(LIBQUADLIB_DEP) variable, that expands to
.../libquadlib.la if that happens to be in-tree (and empty if LIBQUADLIB
is -lquadlib)?

>  myexeclib_LTLIBRARIES = libgfortranbegin.la
>  myexeclibdir = $(libdir)/gcc/$(target_alias)/$(gcc_version)$(MULTISUBDIR)

> --- a/libgfortran/acinclude.m4
> +++ b/libgfortran/acinclude.m4
> @@ -275,3 +275,65 @@ esac])
>      AC_DEFINE(HAVE_BROKEN_POWF, 1, [Define if powf is broken.])
>    fi
>  ])
> +
> +dnl Check whether we have a __float128 type
> +AC_DEFUN([LIBGFOR_CHECK_FLOAT128], [
> +  LIBQUADSPEC=
> +  AC_CACHE_CHECK([whether we have a usable __float128 type],
> +                 libgfor_cv_have_float128, [
> +    AC_TRY_LINK([
> +/* no header */
> +],[
> +  typedef _Complex float __attribute__((mode(TC))) __complex128;
> +
> +  __float128 x;
> +  x = __builtin_huge_valq() - 2.e1000Q;
> +
> +  __complex128 z1, z2;
> +  z1 = x;
> +  z2 = 2.Q;
> +
> +  z1 /= z2;
> +  z1 /= 7.Q;
> +],
> +    eval "libgfor_cv_have_float128=yes",
> +    eval "libgfor_cv_have_float128=no")

No need for eval and double-quotes in these two lines.

> +  ])
> +
> +  if test "x$libgfor_cv_have_float128" = xyes; then
> +    dnl Check whether -Wl,--as-needed is supported
> +    dnl 
> +    dnl Turn warnings into error to avoid testsuite breakage.  So enable
> +    dnl AC_LANG_WERROR, but there's currently (autoconf 2.64) no way to turn
> +    dnl it off again.  As a workaround, save and restore werror flag like
> +    dnl AC_PATH_XTRA.
> +    dnl Cf. http://gcc.gnu.org/ml/gcc-patches/2010-05/msg01889.html
> +    ac_xsave_[]_AC_LANG_ABBREV[]_werror_flag=$ac_[]_AC_LANG_ABBREV[]_werror_flag

The test that follows lacks commands for stdout output (so the user
knows what's being tested) and caching (so the user may override a test
that is broken).  I assume that is also the case in the place where you
copied it from.  Anyway, way to go is to wrap the actual test:

       AC_CACHE_CHECK([whether --as-needed works],
         [libgfor_cv_have_as_needed],
         [

> +    save_LDFLAGS="$LDFLAGS"
> +    LDFLAGS="$LDFLAGS -Wl,--as-needed -lm -Wl,--no-as-needed"
> +    libgfor_cv_have_as_needed=no
> +    AC_LANG_WERROR
> +    AC_LINK_IFELSE([int main(void){ return 0;} ],

       AC_LINK_IFELSE([AC_LANG_PROGRAM([])],


> +		      [eval "libgfor_cv_have_as_needed=yes"],
> + 		      [eval "libgfor_cv_have_as_needed=no"])

eval and double-quotes unneeded.

> +    LDFLAGS="$save_LDFLAGS"
> +    ac_[]_AC_LANG_ABBREV[]_werror_flag=$ac_xsave_[]_AC_LANG_ABBREV[]_werror_flag

Insert   ])  here.

> +    AC_DEFINE(HAVE_FLOAT128, 1, [Define if have a usable __float128 type.])

FWIW, I'd remove this line up to be the first after
    if test "x$libgfor_cv_have_float128"

because it is not related to the as-needed test.

> +    dnl For static libgfortran linkage, depend on libquadmath only if needed.
> +    if test "x$libgfor_cv_have_as_needed" = xyes; then
> +      LIBQUADSPEC="%{static-libgfortran:--as-needed} -lquadmath %{static-libgfortran:--no-as-needed}"
> +    else
> +      LIBQUADSPEC="-lquadmath"
> +    fi
> +    LIBQUADLIB="../libquadmath/libquadmath.la"
> +  fi

You could set LIBQUADLIB_DEP here, as stated above.

> +  dnl For the spec file
> +  AC_SUBST(LIBQUADSPEC)
> +  AC_SUBST(LIBQUADLIB)
> +
> +  dnl We need a conditional for the Makefile
> +  AM_CONDITIONAL(LIBGFOR_BUILD_QUAD, [test "x$libgfor_cv_have_float128" = xyes])
> +])
> diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
> index e5517a1..47315d5 100644
> --- a/libgfortran/configure.ac
> +++ b/libgfortran/configure.ac
> @@ -111,6 +111,9 @@ esac
>  AC_SUBST(toolexecdir)
>  AC_SUBST(toolexeclibdir)
>  
> +# Create a spec file, so that compile/link tests don't fail
> +test -f libgfortran.spec || touch libgfortran.spec
> +
>  # Check the compiler.
>  # The same as in boehm-gc and libstdc++. Have to borrow it from there.
>  # We must force CC to /not/ be precious variables; otherwise
> @@ -459,6 +462,9 @@ LIBGFOR_CHECK_MINGW_SNPRINTF
>  # Check for a broken powf implementation
>  LIBGFOR_CHECK_FOR_BROKEN_POWF
>  
> +# Check whether we have a __float128 type
> +LIBGFOR_CHECK_FLOAT128
> +
>  # Check for GNU libc feenableexcept
>  AC_CHECK_LIB([m],[feenableexcept],[have_feenableexcept=yes AC_DEFINE([HAVE_FEENABLEEXCEPT],[1],[libm includes feenableexcept])])
>  
> @@ -509,6 +515,9 @@ else
>    multilib_arg=
>  fi
>  
> -# Write our Makefile.
> -AC_CONFIG_FILES(Makefile)
> +# Write our Makefile and spec file.
> +AC_CONFIG_FILES([
> +Makefile
> +libgfortran.spec
> +])
>  AC_OUTPUT

> --- a/libgomp/configure.ac
> +++ b/libgomp/configure.ac
> @@ -147,15 +147,17 @@ case `echo $GFORTRAN` in
>    -* | no* )
>      FC=no ;;
>    *)
> -    set dummy $GFORTRAN; ac_word=$2
> +    set dummy $GFORTRAN; ac_word="$2 -L ../libgfortran"
>      if test -x "$ac_word"; then

This isn't right.  'test -x' tests whether a file is executable.  You
should only pass it a file name here; simply dropping this hunk seems
right to me.

>        FC="$GFORTRAN"
>      else
>        FC=no
>      fi ;;
>  esac
> +
> +# The -L../libgfortran is needed for to find libgfortran.spec
>  AC_PROG_FC(gfortran)
> -FCFLAGS="$FCFLAGS -Wall"
> +FCFLAGS="$FCFLAGS -Wall -L ../libgfortran"

No space after -L

>  # For libtool versioning info, format is CURRENT:REVISION:AGE
>  libtool_VERSION=1:0:0

Cheers,
Ralf


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]