gcc optimisation breaks guile floating point

gcc optimisation breaks guile floating point

[Gary Houston]

Floating point seems somewhat broken in latest CVS if compiled with
recent gcc (e.g., version 3.2):

guile> 3.2
0.0

The problem is in libguile/numbers.c:scm_make_real, and only if
scm_double_cell is inlined (so it isn't a problem in Guile 1.5).

Compiling with the -fno-strict-aliasing option fixes it.  Here's a
similar kind of problem code:

#include <stdlib.h>
#include <string.h>

static void * make_z (double x)
{
  void *z = malloc (50);

  memset (z, 0, 50);
  *((unsigned long *) z) = 0;
  *((double *) z) = x;
  return z;
}

main ()
{
  void *z = make_z (3.2);
  double d = *((double *) z);
  int i;

  for (i = 0; i < sizeof (double); i++)
    {
      printf ("%d\n", ((unsigned char *) &d)[i]);
    }
  return 0;
}

Apparently, "according to ANSI C" (no direct quotation available), the
compiler can assume that *((unsigned long *) z) and *((double *) z)
refer to different locations and reorder the statements!

I don't know what would be needed to make Guile conform to such
stringent interpretation of the standard, but I don't think it would
be easy to determine that it was correct and that it would always stay
that way.  Perhaps we should just add the -fno-strict-aliasing option
if the compiler supports it?



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

gcc optimisation breaks guile floating point

[Han-Wen Nienhuys]

ghouston@arglist.com writes:
> 
> Apparently, "according to ANSI C" (no direct quotation available), the
> compiler can assume that *((unsigned long *) z) and *((double *) z)
> refer to different locations and reorder the statements!
> 
> I don't know what would be needed to make Guile conform to such
> stringent interpretation of the standard, but I don't think it would
> be easy to determine that it was correct and that it would always stay
> that way.  Perhaps we should just add the -fno-strict-aliasing option
> if the compiler supports it?

I think that these double (as in twice) initializations are weird, and
should go. Why not

    scm_t_double bla;
    bla.tag = scm_double_tag
    bla.pad = 0x0
    bla.double = z;
    return scm_double_cell (
        ((scm_t_double_cell*) &bla) -> car,
        ((scm_t_double_cell*) &bla) -> cbr,
        ((scm_t_double_cell*) &bla) -> ccr,
        ((scm_t_double_cell*) &bla) -> cdr);

If we use -fno-strict-aliasing (a kludge, IMO), we should double check
that all inline functions are correct , for clients might not compile
with -fno-strict-aliasing.

Another option

  *((unsigned long *) z) = 0;
  scm_remember ((SCM) z);
  *((double *) z) = x;
  return z;


-- 

Han-Wen Nienhuys   |   hanwen@cs.uu.nl   |   http://www.cs.uu.nl/~hanwen 


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Marius Vollmer]

Han-Wen Nienhuys <hanwen@cs.uu.nl> writes:

> I think that these double (as in twice) initializations are weird, and
> should go. Why not
> 
>     scm_t_double bla;
>     bla.tag = scm_double_tag
>     bla.pad = 0x0
>     bla.double = z;
>     return scm_double_cell (
>         ((scm_t_double_cell*) &bla) -> car,
>         ((scm_t_double_cell*) &bla) -> cbr,
>         ((scm_t_double_cell*) &bla) -> ccr,
>         ((scm_t_double_cell*) &bla) -> cdr);

This looks wierd as well, don't you think?

> Another option
> 
>   *((unsigned long *) z) = 0;
>   scm_remember ((SCM) z);
>   *((double *) z) = x;
>   return z;

I like this better (with a suitable comment of course).

I guess that the funny aliasing behaviour applies only to pointers to
floating point types, right?

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Han-Wen Nienhuys]

mvo@zagadka.ping.de writes:
> 
> This looks wierd as well, don't you think?

well, yes, probably.

> >   *((unsigned long *) z) = 0;
> >   scm_remember ((SCM) z);
> >   *((double *) z) = x;
> >   return z;
> 
> I like this better (with a suitable comment of course).

In CVS.

> I guess that the funny aliasing behaviour applies only to pointers to
> floating point types, right?

I wouldn't be sure. It could happen with all sorts of initializations,
I guess, but I'm not familiar enough with the source code to see which
ones.


-- 
Han-Wen Nienhuys   |   hanwen@cs.uu.nl   |   http://www.cs.uu.nl/~hanwen 


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Gary Houston]

> From: Han-Wen Nienhuys <hanwen@cs.uu.nl>
> Date: Thu, 29 Aug 2002 00:53:57 +0200
> 
> > >   *((unsigned long *) z) = 0;
> > >   scm_remember ((SCM) z);
> > >   *((double *) z) = x;
> > >   return z;
> > 
> > I like this better (with a suitable comment of course).
> 
> In CVS.

Thanks, that's better:

guile> 3.2
3.2

However scm_double_cell itself probably needs to be fixed, otherwise
this problem will turn up again some day, or in user code (I'll look
at it myself if it's still a problem in a couple of weeks.)

> > I guess that the funny aliasing behaviour applies only to pointers to
> > floating point types, right?
> 
> I wouldn't be sure. It could happen with all sorts of initializations,
> I guess, but I'm not familiar enough with the source code to see which
> ones.

I've read that it happens whenever the types differ significantly,
e.g., int vs double or int vs struct both have the problem.  I wonder
if two different struct types would cause it, e.g., the old trick:

struct foo
{
  struct bar
  {
  }
  ...
}

with casting of pointers between foo and bar objects.



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Han-Wen Nienhuys]

ghouston@arglist.com writes:
> 
> However scm_double_cell itself probably needs to be fixed, otherwise
> this problem will turn up again some day, or in user code (I'll look
> at it myself if it's still a problem in a couple of weeks.)

I think the proper solution is some kind of asm/gcc statement that
prevents the reordering. Any C gurus that know a portable way of
ensuring that?  A function call like scm_remember() from
scm_[double_]cell() defeats the purpose of inlining scm_[double_]cell. 

-- 

Han-Wen Nienhuys   |   hanwen@cs.uu.nl   |   http://www.cs.uu.nl/~hanwen 


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Carl R. Witty]

Han-Wen Nienhuys <hanwen@cs.uu.nl> writes:

> ghouston@arglist.com writes:
> > 
> > However scm_double_cell itself probably needs to be fixed, otherwise
> > this problem will turn up again some day, or in user code (I'll look
> > at it myself if it's still a problem in a couple of weeks.)
> 
> I think the proper solution is some kind of asm/gcc statement that
> prevents the reordering. Any C gurus that know a portable way of
> ensuring that?  A function call like scm_remember() from
> scm_[double_]cell() defeats the purpose of inlining scm_[double_]cell. 

Under gcc, the following statement:

  asm volatile ("" : : : "memory");

is approximately what you want.  This specifies the empty string as
inline assembly (so nothing is actually inserted into the assembly
output), but tells gcc that the instruction may read and write memory
in arbitrary ways.  Thus, gcc will not reorder memory reads or writes
past this statement.

Of course, this does have other undesirable effects on optimization --
gcc cannot reorder other, unrelated memory reads or writes past this
statement either, and it also cannot keep values from memory cached in
registers across the statement.  It also only works with gcc.

I'm unfamiliar with the exact definition of ANSI C, but I wonder if
something like this might be correct:

typedef union {
	long l;
	double d;
} long_or_double;

...
  *((long_or_double *)z).l = 0;
  *((long_or_double *)z).d = x;

Carl Witty


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Han-Wen Nienhuys]

cwitty@newtonlabs.com writes:
> I'm unfamiliar with the exact definition of ANSI C, but I wonder if
> something like this might be correct:
> 
> typedef union {
> 	long l;
> 	double d;
> } long_or_double;
> 
> ...
>   *((long_or_double *)z).l = 0;
>   *((long_or_double *)z).d = x;

Yes, but it  doesn't jibe  with the  prototype of scm_double_cell(),
which is

      SCM scm_double_cell(SCM,SCM,SCM,SCM);

-- 

Han-Wen Nienhuys   |   hanwen@cs.uu.nl   |   http://www.cs.uu.nl/~hanwen 


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Gary Houston]

> From: cwitty@newtonlabs.com (Carl R. Witty)
> Date: 11 Sep 2002 18:46:47 -0700
> 
> Han-Wen Nienhuys <hanwen@cs.uu.nl> writes:
> 
> > ghouston@arglist.com writes:
> > > 
> > > However scm_double_cell itself probably needs to be fixed, otherwise
> > > this problem will turn up again some day, or in user code (I'll look
> > > at it myself if it's still a problem in a couple of weeks.)
> > 
> > I think the proper solution is some kind of asm/gcc statement that
> > prevents the reordering. Any C gurus that know a portable way of
> > ensuring that?  A function call like scm_remember() from
> > scm_[double_]cell() defeats the purpose of inlining scm_[double_]cell. 
> 
> Under gcc, the following statement:
> 
>   asm volatile ("" : : : "memory");
> 
> is approximately what you want.  This specifies the empty string as
> inline assembly (so nothing is actually inserted into the assembly
> output), but tells gcc that the instruction may read and write memory
> in arbitrary ways.  Thus, gcc will not reorder memory reads or writes
> past this statement.

This is good, it's similar to a previous work-around in libguile/throw.c.

> Of course, this does have other undesirable effects on optimization --
> gcc cannot reorder other, unrelated memory reads or writes past this
> statement either, and it also cannot keep values from memory cached in
> registers across the statement.  It also only works with gcc.

Judging by the assembler output, it doesn't seem to do much damage.
For other compilers, calling scm_remember_upto_here_1 will avoid the
problem, with slightly greater overhead perhaps.

So I've moved the work-around into scm_double_cell itself.



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Marius Vollmer]

Gary Houston <ghouston@arglist.com> writes:

> > Under gcc, the following statement:
> > 
> >   asm volatile ("" : : : "memory");
> >
> > [...]
>
> This is good, it's similar to a previous work-around in libguile/throw.c.

I haven't checked with the standard, but it might be the case that a
compiler must assume that a pointer to char can alias any type.  Thus, 

  #define SCM_REAL_VALUE(x) (((scm_t_double *)((char *)SCM2PTR (x)))->real)

might work as well.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Gary Houston]

> Cc: cwitty@newtonlabs.com, hanwen@cs.uu.nl, guile-devel@gnu.org
> From: Marius Vollmer <mvo@zagadka.ping.de>
> Date: 03 Oct 2002 00:40:24 +0200
> 
> Gary Houston <ghouston@arglist.com> writes:
> 
> > > Under gcc, the following statement:
> > > 
> > >   asm volatile ("" : : : "memory");
> > >
> > > [...]
> >
> > This is good, it's similar to a previous work-around in libguile/throw.c.
> 
> I haven't checked with the standard, but it might be the case that a
> compiler must assume that a pointer to char can alias any type.  Thus, 
> 
>   #define SCM_REAL_VALUE(x) (((scm_t_double *)((char *)SCM2PTR (x)))->real)
> 
> might work as well.

I tried the extra cast on the simple test case, but it doesn't help.
In ANSI C any such assumption would be for void *, not char *, but
the pointer was void * already.

#include <stdlib.h>
#include <string.h>

static void * make_z (double x)
{
  void *z = malloc (50);

  memset (z, 0, 50);
  *((unsigned long *) z) = 0;
  *((double *) z) = x;
  return z;
}

main ()
{
  void *z = make_z (3.2);
  double d = *((double *) z);
  int i;

  for (i = 0; i < sizeof (double); i++)
    {
      printf ("%d\n", ((unsigned char *) &d)[i]);
    }
  return 0;
}




_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: gcc optimisation breaks guile floating point

[Marius Vollmer]

Gary Houston <ghouston@arglist.com> writes:

> I tried the extra cast on the simple test case, but it doesn't help.

Ahh, too bad.  I got the idea from this item in the GCC 3.3 changes:

  * A new type attribute, may_alias, has been added. Accesses to
    objects with types with this attribute are not subjected to
    type-based alias analysis, but are instead assumed to be able to
    alias any other type of objects, just like the char type.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel