Created attachment 21688 [details]
gcc/config.log from x86_64-apple-darwin10

Created attachment 21689 [details]
gcc/autohost.h from x86_64-apple-darwin10

Added gcc/config.log and gcc/autohost.h files from a build on x86_64-apple-darwin10 configured with...

../gcc/configure --prefix=/Users/howarth/dist --with-gmp=/sw --with-libiconv-prefix=/sw --with-ppl=/sw --with-cloog=/sw --with-mpc=/sw --with-system-zlib --x-includes=/usr/X11R6/include --x-libraries=/usr/X11R6/lib --enable-checking=yes --enable-languages=c --enable-lto

Looking at a build previous to r163815/r163816, I find that config.log has...

enable_decimal_float='no'

in the libgcc build subdirectories, but config.log shows...

enable_decimal_float='dpd'

everywhere else in the build tree. Now after r163815/r163816, enable_decimal_float='dpd'  appears in config.log of the libgcc subdirectory as well.

Okay the problem is that pre-r163815/r163816, we had in gcc/configure.ac...

# x86's use BID format instead of DPD
case x$enable_decimal_float in
  xyes)
    case $target in
      i?86*-*-linux* | x86_64*-*-linux*)
        enable_decimal_float=bid
        ;;
      *)
        enable_decimal_float=dpd
        ;;
    esac
    ;;
  xno)
    # ENABLE_DECIMAL_FLOAT is set to 0. But we have to have proper
    # dependency on libdecnumber.
    enable_decimal_float=dpd
    ;;
esac

whereas in libgcc/configure.ac, we had a special case of just...

# x86's use BID format instead of DPD
if test x$enable_decimal_float = xyes; then
  case $host in
    i?86*-*-linux* | x86_64*-*-linux*)
      enable_decimal_float=bid
      ;;
    *)
      enable_decimal_float=dpd
      ;;
  esac
fi

that did nothing in the case of enable_decimal_float=no.

Our problem is that dfp.m4 is too coarse of a solution as it forces the same test to be used everywhere...


# x86's use BID format instead of DPD
case x$enable_decimal_float in
  xyes)
    case $target in
      i?86*-*-* | x86_64*-*-*)
        enable_decimal_float=bid
        ;;
      *)
        enable_decimal_float=dpd
        ;;
    esac
    ;;
  xno)
    # ENABLE_DECIMAL_FLOAT is set to 0. But we have to have proper
    # dependency on libdecnumber.
    enable_decimal_float=dpd
    ;;
esac

and invalidly sets enable_decimal_float=dpd for libgcc.

Of course this bug impacts all non-linux targets so it should be a P1.

Apparently the configure option '--enable-decimal-float=no' is not even working:


[macbook] f90/bug% gfc -v
Using built-in specs.
COLLECT_GCC=gfc
COLLECT_LTO_WRAPPER=/opt/gcc/gcc4.6w/libexec/gcc/x86_64-apple-darwin10.4.0/4.6.0/lto-wrapper
Target: x86_64-apple-darwin10.4.0
Configured with: ../work/configure --prefix=/opt/gcc/gcc4.6w --enable-languages=c,c++,fortran,objc,obj-c++,java,lto --with-gmp=/opt/sw64 --with-libiconv-prefix=/opt/sw64 --with-system-zlib --with-cloog=/opt/sw64 --enable-decimal-float=no
Thread model: posix
gcc version 4.6.0 20100903 (experimental) [trunk revision 163847p4] (GCC) 
[macbook] gcc/build_w% grep -r enable_decimal_float */config.log
gcc/config.log:enable_decimal_float='dpd'
libdecnumber/config.log:enable_decimal_float='dpd'
prev-gcc/config.log:enable_decimal_float='dpd'
prev-libdecnumber/config.log:enable_decimal_float='dpd'
stage1-gcc/config.log:enable_decimal_float='dpd'
stage1-libdecnumber/config.log:enable_decimal_float='dpd'

Reverting and regenerating libgcc/configure.ac is insufficient to eliminate the failures. I will try reverting both libgcc/configure.ac and libdecnumber/configure.ac next. I suspect the changes in r163815/r163816  are misguided in that it wasn't appreciated how the behavior of dfp.m4 had to be modulated for each location. This being the case, the advantage of merging these configure.ac code into a single m4 file seems nullified.

To make clearer the subtleties being lost in a single dfp.m4 file, here are the diffs of the lines replaced out of configure.ac from each location (gcc, libgcc and libdecnumber).

--- gcc	2010-09-03 22:04:53.000000000 -0400
+++ libgcc	2010-09-03 22:01:16.000000000 -0400
@@ -11,34 +11,26 @@
   esac
 ],
 [
-  case $target in
+  case $host in
     powerpc*-*-linux* | i?86*-*-linux* | x86_64*-*-linux*)
       enable_decimal_float=yes
       ;;
     *)
-      AC_MSG_WARN(decimal float is not supported for this target, ignored)
       enable_decimal_float=no
       ;;
   esac
 ])
+
 # x86's use BID format instead of DPD
-case x$enable_decimal_float in
-  xyes)
-    case $target in
-      i?86*-*-linux* | x86_64*-*-linux*)
-        enable_decimal_float=bid
-        ;;
-      *)
-        enable_decimal_float=dpd
-        ;;
-    esac
-    ;;
-  xno)
-    # ENABLE_DECIMAL_FLOAT is set to 0. But we have to have proper
-    # dependency on libdecnumber.
-    enable_decimal_float=dpd
-    ;;
-esac
+if test x$enable_decimal_float = xyes; then
+  case $host in
+    i?86*-*-linux* | x86_64*-*-linux*)
+      enable_decimal_float=bid
+      ;;
+    *)
+      enable_decimal_float=dpd
+      ;;
+  esac
+fi
 AC_SUBST(enable_decimal_float)
 
-

--- gcc	2010-09-03 22:04:53.000000000 -0400
+++ libdecnumber	2010-09-03 21:59:43.000000000 -0400
@@ -1,3 +1,6 @@
+# Default decimal format
+# If you change the defaults here, be sure to change them in the GCC directory also
+AC_MSG_CHECKING([for decimal floating point])
 AC_ARG_ENABLE(decimal-float,
 [  --enable-decimal-float={no,yes,bid,dpd}
                         enable decimal float extension to C.  Selecting 'bid'
@@ -16,29 +19,22 @@
       enable_decimal_float=yes
       ;;
     *)
-      AC_MSG_WARN(decimal float is not supported for this target, ignored)
       enable_decimal_float=no
       ;;
   esac
 ])
-# x86's use BID format instead of DPD
-case x$enable_decimal_float in
-  xyes)
-    case $target in
-      i?86*-*-linux* | x86_64*-*-linux*)
-        enable_decimal_float=bid
-        ;;
-      *)
-        enable_decimal_float=dpd
-        ;;
-    esac
-    ;;
-  xno)
-    # ENABLE_DECIMAL_FLOAT is set to 0. But we have to have proper
-    # dependency on libdecnumber.
-    enable_decimal_float=dpd
-    ;;
-esac
-AC_SUBST(enable_decimal_float)
 
+# x86's use BID format instead of DPD
+# In theory --enable-decimal-float=no should not compile anything
+# For the sake of simplicity, just use the default format in this directory
+if test x$enable_decimal_float = xyes -o x$enable_decimal_float = xno; then
+  case $target in
+    i?86*-*-linux* | x86_64*-*-linux*)
+      enable_decimal_float=bid
+      ;;
+    *)
+      enable_decimal_float=dpd
+      ;;
+  esac
+fi
 

(In reply to comment #9)> 
> --- gcc 2010-09-03 22:04:53.000000000 -0400
> +++ libgcc      2010-09-03 22:01:16.000000000 -0400
> @@ -11,34 +11,26 @@
>    esac
>  ],
>  [
> -  case $target in
> +  case $host in

This is correct as libgcc is a target library so the host there is what we originally had as the target.

I am not worried about $host vs $target but rather the nuances in the xno case for x$enable_decimal_float in the original code. For gcc/configure.ac, this sets...

enable_decimal_float=dpd

but that case is not handled in gcc/configure.ac. Likewise comparing gcc/configure.ac to libdecnumber.ac,that case is also ignored but there also is a wrapping test around the entire case statement of...

if test x$enable_decimal_float = xyes -o x$enable_decimal_float = xno; then

these subtleties have not been extended into dfp.m4 nor has the underlying reason for them been addressed.

(In reply to comment #11)

> but that case is not handled in gcc/configure.ac. Likewise comparing


but that case is not handled in libgcc/configure.ac. Likewise comparing

Reverting both libgcc/configure.ac and libdecnumber/configure.ac and regenerating these files is insufficient as well to eliminate the regressions on darwin. Looking at the remaining changes in gcc/configure.ac it is obvious way this change badly breaks non-linux targets.

http://gcc.gnu.org/viewcvs/trunk/gcc/configure.ac?r1=163815&r2=163814&pathrev=163815

The author cut out two sections from gcc/configure.ac and pasted them together in dfp.m4. The intervening section...

 dfp=`if test $enable_decimal_float != no; then echo 1; else echo 0; fi`
 AC_DEFINE_UNQUOTED(ENABLE_DECIMAL_FLOAT, $dfp,
 [Define to 1 to enable decimal float extension to C.])

is now moved to the end, breaking the original logic. This is without even considering the other subtleties in the same code from libgcc/configure.ac and libdecnumber/configure.ac that has been simply ignored.

The original patch introduce this minor variations in the code copied for dfp.m4 in the gcc/configure.ac, libgcc/configure.ac and libdecnumber.ac is found here...

http://gcc.gnu.org/ml/gcc-patches/2007-03/msg01608.html

and committed here...

http://gcc.gnu.org/ml/gcc-cvs/2007-03/msg00776.html

Please revert the patch in both gcc and src.  It's clear that the way to go is to first write small patch to smooth out the nuances you pointed out, and then introduce the new macro in a way that doesn't change the semantics.  Sorry for the spotty review. :/

Could someone check that revisions 163815 and 163816 are not exposing a more serious latent bug: I have configured gcc with --enable-decimal-float=no and --disable-decimal-float without disabling -decimal-float. 

Are these options working on linux?

Subject: Re:  r163815/r163816 produces new regressions on
 x86_64-apple-darwin10

On Sat, 4 Sep 2010, bonzini at gnu dot org wrote:

> Please revert the patch in both gcc and src.  It's clear that the way to go is
> to first write small patch to smooth out the nuances you pointed out, and then
> introduce the new macro in a way that doesn't change the semantics.  Sorry for
> the spotty review. :/

Since the differences are generally deliberate, what's actually wanted is 
a macro that preserves them.  The basic information of which targets 
support DFP by default / at all and what the default format is on each 
target should be in a single place.  The information about whether $target 
or $host is relevant, and about what the configuration variables should be 
set to if DFP is disabled, should be passed as arguments to the macro.

See also http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00342.html for a non-darwin platform.

(In reply to comment #18)
> See also http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00342.html for a
> non-darwin platform.
> 

  Yep, it's all the same kind of undefined symbol problems as in Jack's original description of the bug.  I configured using plain --enable-decimal-float, I imagine that explicitly specifying --enable-decimal-float=bid would help.  (Shall give it a try.)

(In reply to comment #19)
> (In reply to comment #18)
> > See also http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00342.html for a
> > non-darwin platform.
> > 
> 
>   Yep, it's all the same kind of undefined symbol problems as in Jack's
> original description of the bug.  

Note however that it's before the revision where this patch was applied, and wouldn't have shown up without my explictly adding the --enable option.

Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 163853)
+++ gcc/configure.ac	(working copy)
@@ -608,10 +608,6 @@
 # Enable C extension for decimal float if target supports it.
 GCC_AC_ENABLE_DECIMAL_FLOAT([$target])
 
-dfp=`if test $enable_decimal_float != no; then echo 1; else echo 0; fi`
-AC_DEFINE_UNQUOTED(ENABLE_DECIMAL_FLOAT, $dfp,
-[Define to 1 to enable decimal float extension to C.])
-
 bid=`if test $enable_decimal_float = bid; then echo 1; else echo 0; fi`
 AC_DEFINE_UNQUOTED(ENABLE_DECIMAL_BID_FORMAT, $bid,
 [Define to 1 to specify that we are using the BID decimal floating

Index: config/dfp.m4
===================================================================
--- config/dfp.m4	(revision 163853)
+++ config/dfp.m4	(working copy)
@@ -30,6 +30,10 @@
   esac
 ])
 
+ dfp=`if test $enable_decimal_float != no; then echo 1; else echo 0; fi`
+ AC_DEFINE_UNQUOTED(ENABLE_DECIMAL_FLOAT, $dfp,
+ [Define to 1 to enable decimal float extension to C.])
+
 # x86's use BID format instead of DPD
 case x$enable_decimal_float in
   xyes)
@@ -50,4 +54,4 @@
 esac
 AC_SUBST(enable_decimal_float)
 
-])
\ No newline at end of file
+])

which restore the original code sequencing, eliminates the decimal float failures on x86_64-apple-darwin10. However, I don't know what this does to linux due to the remaining differences being omitted.

I should note that one probe I am using to monitor how this builds is what "grep decimal_float config.log" reports in the gcc, libdecnumber and libgcc build directories. Pre-r163815/r163816, on darwin this was..

enable_decimal_float='dpd'

for gcc and libdecnumber but...

decimal_float='no'
enable_decimal_float='no'

in the libgcc build directory. With the patch in comment 21, this is changed to...

decimal_float='no'
enable_decimal_float='dpd'

Note that this compares to what we got in libgcc with r163815/r163816 of...

decimal_float='yes'
enable_decimal_float='dpd'

The patch in comment 21 seems to work on x86_64-unknown-linux-gnu for a stock build. Iin the libgcc build directory before and after the patch, I get...

decimal_float='yes'
enable_decimal_float='bid'

Created attachment 21695 [details]
Move definition of ENABLE_DECIMAL_FLOAT to 1 into config/dfp.m4.

Created attachment 21696 [details]
A patch

Please try this one.

Created attachment 21697 [details]
A regenerated patch

I really don't like autconf cache.

Both patches fail on a x86_64-apple-darwin10 bootstrap with..

make[3]: *** No rule to make target `../../gcc-4.6-20100904/gcc/../libdecnumber/no/decimal32.h', needed by `dfp.o'.  Stop

Created attachment 21698 [details]
A new patch

Try this one.

(In reply to comment #28)
> Created an attachment (id=21698) [edit]
> A new patch
> 
> Try this one.
> 

This bootstraps. Regtesting next.

> > It's clear that the way to go is to first write
> > small patch to smooth out the nuances you pointed out, and then
> > introduce the new macro in a way that doesn't change the semantics.
> 
> Since the differences are generally deliberate, what's actually wanted is 
> a macro that preserves them.

Sure, I meant "leaving the differences in semantics to the single configure scripts".  This is what H.J.'s patch does more or less, and I like it.

I'm not sure whether the patches remove the need for the other patch in comment #21 though.  Jack, can you post what you are exactly testing (without the regenerated files, which are unnecessary)?

(In reply to comment #30)

> I'm not sure whether the patches remove the need for the other patch in comment
> #21 though.  Jack, can you post what you are exactly testing (without the
> regenerated files, which are unnecessary)?
> 

Testing H.J.'s gcc-pr45524-3.patch as it is the first in his series which bootstraps on x86_64-apple-darwin10.

I can confirm that gcc-pr45524-3.patch bootstraps and eliminates the regressions caused by r163815/r163816 on x86_64-apple-darwin10.

A patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00375.html

Regression result for gcc-pr45524-3.patch on x86_64-apple-darwin10.

http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00410.html

*** Bug 45547 has been marked as a duplicate of this bug. ***

I confirm that the patch in comment #28 fixes this pr. However using the tip in comment #22, I get

[macbook] gcc/p_build% grep -r decimal_ */config.log
gcc/config.log:enable_decimal_float='dpd'
libdecnumber/config.log:enable_decimal_float='dpd'
prev-gcc/config.log:enable_decimal_float='dpd'
prev-libdecnumber/config.log:enable_decimal_float='dpd'
stage1-gcc/config.log:enable_decimal_float='dpd'
stage1-libdecnumber/config.log:enable_decimal_float='dpd'

Is there a better way to know if decimal_float has been installed or not?

(In reply to comment #36)
> I confirm that the patch in comment #28 fixes this pr. However using the tip in
> comment #22, I get
> 
> [macbook] gcc/p_build% grep -r decimal_ */config.log
> gcc/config.log:enable_decimal_float='dpd'
> libdecnumber/config.log:enable_decimal_float='dpd'
> prev-gcc/config.log:enable_decimal_float='dpd'
> prev-libdecnumber/config.log:enable_decimal_float='dpd'
> stage1-gcc/config.log:enable_decimal_float='dpd'
> stage1-libdecnumber/config.log:enable_decimal_float='dpd'
> 

This how DFP is implemented.

(In reply to comment #33)
> A patch is posted at
> 
> http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00375.html
> 
Thanks for fixing it. And sorry for not testing it thoroughly.

Subject: Bug 45524

Author: hjl
Date: Mon Sep  6 14:52:54 2010
New Revision: 163921

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=163921
Log:
Don't set enable_decimal_float to dpd if DFP is disabled.

config/

2010-09-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/45524
	* dfp.m4: Don't set enable_decimal_float to dpd if DFP is
	disabled.  Set default_decimal_float.

gcc/

2010-09-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/45524
	* configure.ac (enable_decimal_float): Set to
	$default_decimal_float.
	* configure: Regenerated.

libdecnumber/

2010-09-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/45524
	* configure.ac (enable_decimal_float): Set to
	$default_decimal_float.
	* configure: Regenerated.

libgcc/

2010-09-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/45524
	* configure: Regenerated.

Modified:
    trunk/config/ChangeLog
    trunk/config/dfp.m4
    trunk/gcc/ChangeLog
    trunk/gcc/configure
    trunk/gcc/configure.ac
    trunk/libdecnumber/ChangeLog
    trunk/libdecnumber/configure
    trunk/libdecnumber/configure.ac
    trunk/libgcc/ChangeLog
    trunk/libgcc/configure

*** Bug 45561 has been marked as a duplicate of this bug. ***

Fixed.