patch: fix double alignment detection on Solaris/SPARC with gcc-4.4.x

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

patch: fix double alignment detection on Solaris/SPARC with gcc-4.4.x

Andrew Gaylard

On Mon, Jul 20, 2009 at 7:02 PM, David T. Lewis<[hidden email]> wrote:

>
> On Sat, Jul 18, 2009 at 05:25:48PM +0200, Andrew Gaylard wrote:
>>
>> The main issue that I'm facing at the moment is that gcc-4.4.0 generates
>> code that causes unaligned accesses in fetchFloatAtinto; the same code
>> works in gcc-4.2.4 on SPARC.  Until I or someone else (hint!) fixes this,
>> I'm continuing to build with gcc-4.2.4.
>
> fetchFloatAtInto() is implemented in platforms/Cross/vm/sqMemoryAccess.h
> in the form of cpp macros. It might be tricky to debug this, but I note
> that the implementation depends on DOUBLE_WORD_ALIGNMENT, which is a macro
> that would have been set when you run configure. You might playing with
> this setting to see if it gives you the results you want. For example, as
> a complete SWAG, it might be the case that SPARC plus gcc-4.2.4 needs this
> macro defined but for some reason configure is not setting it for you.

David, thanks for the tip.  This message from configure showed
you are right (the 'yes' should be 'no'):

checking whether unaligned access to doubles is ok... yes

It turns out that newer GCCs, from 4.3.x onward, are smart
enough to optimise out the code used to detect strict alignment:

bash-3.2$ gcc -O2 double-test.c
double-test.c: In function 'main':
double-test.c:6: warning: passing argument 1 of 'f' makes integer from
pointer without a cast
double-test.c:1: note: expected 'int' but argument is of type 'char *'
bash-3.2$ ./a.out
bash-3.2$ echo $?
0

But when compiling without optimisation:

bash-3.2$ gcc double-test.c
double-test.c: In function 'main':
double-test.c:6: warning: passing argument 1 of 'f' makes integer from
pointer without a cast
double-test.c:1: note: expected 'int' but argument is of type 'char *'
bash-3.2$ ./a.out
Bus Error (core dumped)

... as expected.

This patch complicates the code sufficiently (by using the results
of the function) so that it triggers a bus error both with and without
optimisation, as would be expected.  Of course, configure needs
to be regenerated after applying this patch.

Running our internal benchmark reveals that it is worth it getting
4.4.1 to work:

gcc-4.4.1:

real    61m45.975s
user    56m16.247s
sys     4m57.253s

gcc-4.2.4:

real    67m29.260s
user    61m40.666s
sys     5m7.937s

This means the VM is sped up by 9%.

- Andrew

--- Squeak-3.10-4/platforms/unix/config/acinclude.m4.orig
2008-09-02 20:49:43.000000000 +0200
+++ Squeak-3.10-4/platforms/unix/config/acinclude.m4    2009-07-30
16:16:55.828929831 +0200
@@ -230,10 +230,33 @@


 AC_DEFUN([AC_C_DOUBLE_ALIGNMENT],
 [AC_CACHE_CHECK([whether unaligned access to doubles is ok],
ac_cv_double_align,
-  AC_TRY_RUN([f(int i){*(double *)i=*(double *)(i+4);}
-              int main(){char b[[12]];f(b);return 0;}],
+  AC_TRY_RUN([
+
+int f(void* i){
+       *(double *)i=*(double *)(i+4);
+       return *(char*)i;
+}
+int main(){
+       char b[[12]];
+       b[[0]]=1;
+       b[[1]]=2;
+       b[[2]]=3;
+       b[[3]]=4;
+       b[[4]]=0;
+       b[[5]]=0;
+       b[[6]]=0;
+       b[[7]]=0;
+       b[[8]]=0;
+       b[[9]]=0;
+       b[[10]]=0;
+       b[[11]]=0;
+
+       return f(b);
+}
+],
+
     ac_cv_double_align="yes", ac_cv_double_align="no"))
 test "$ac_cv_double_align" = "no" && AC_DEFINE(DOUBLE_WORD_ALIGNMENT)])

 AC_DEFUN([AC_C_DOUBLE_ORDER],
Reply | Threaded
Open this post in threaded view
|

Re: patch: fix double alignment detection on Solaris/SPARC with gcc-4.4.x

David T. Lewis
 
Excellent, thanks for the follow up Andrew.

Ian,

There is a patch for platforms/unix/config/acinclude.m4 at the end of this
email that addresses an issue for newer GCCs on SPARC. When you get a chance,
can you please update the acinclude.m4 and do a make configure?

Thanks,

Dave

On Thu, Jul 30, 2009 at 10:47:23PM +0200, Andrew Gaylard wrote:

>
> On Mon, Jul 20, 2009 at 7:02 PM, David T. Lewis<[hidden email]> wrote:
> >
> > On Sat, Jul 18, 2009 at 05:25:48PM +0200, Andrew Gaylard wrote:
> >>
> >> The main issue that I'm facing at the moment is that gcc-4.4.0 generates
> >> code that causes unaligned accesses in fetchFloatAtinto; the same code
> >> works in gcc-4.2.4 on SPARC. ?Until I or someone else (hint!) fixes this,
> >> I'm continuing to build with gcc-4.2.4.
> >
> > fetchFloatAtInto() is implemented in platforms/Cross/vm/sqMemoryAccess.h
> > in the form of cpp macros. It might be tricky to debug this, but I note
> > that the implementation depends on DOUBLE_WORD_ALIGNMENT, which is a macro
> > that would have been set when you run configure. You might playing with
> > this setting to see if it gives you the results you want. For example, as
> > a complete SWAG, it might be the case that SPARC plus gcc-4.2.4 needs this
> > macro defined but for some reason configure is not setting it for you.
>
> David, thanks for the tip.  This message from configure showed
> you are right (the 'yes' should be 'no'):
>
> checking whether unaligned access to doubles is ok... yes
>
> It turns out that newer GCCs, from 4.3.x onward, are smart
> enough to optimise out the code used to detect strict alignment:
>
> bash-3.2$ gcc -O2 double-test.c
> double-test.c: In function 'main':
> double-test.c:6: warning: passing argument 1 of 'f' makes integer from
> pointer without a cast
> double-test.c:1: note: expected 'int' but argument is of type 'char *'
> bash-3.2$ ./a.out
> bash-3.2$ echo $?
> 0
>
> But when compiling without optimisation:
>
> bash-3.2$ gcc double-test.c
> double-test.c: In function 'main':
> double-test.c:6: warning: passing argument 1 of 'f' makes integer from
> pointer without a cast
> double-test.c:1: note: expected 'int' but argument is of type 'char *'
> bash-3.2$ ./a.out
> Bus Error (core dumped)
>
> ... as expected.
>
> This patch complicates the code sufficiently (by using the results
> of the function) so that it triggers a bus error both with and without
> optimisation, as would be expected.  Of course, configure needs
> to be regenerated after applying this patch.
>
> Running our internal benchmark reveals that it is worth it getting
> 4.4.1 to work:
>
> gcc-4.4.1:
>
> real    61m45.975s
> user    56m16.247s
> sys     4m57.253s
>
> gcc-4.2.4:
>
> real    67m29.260s
> user    61m40.666s
> sys     5m7.937s
>
> This means the VM is sped up by 9%.
>
> - Andrew
>
> --- Squeak-3.10-4/platforms/unix/config/acinclude.m4.orig
> 2008-09-02 20:49:43.000000000 +0200
> +++ Squeak-3.10-4/platforms/unix/config/acinclude.m4    2009-07-30
> 16:16:55.828929831 +0200
> @@ -230,10 +230,33 @@
>
>
>  AC_DEFUN([AC_C_DOUBLE_ALIGNMENT],
>  [AC_CACHE_CHECK([whether unaligned access to doubles is ok],
> ac_cv_double_align,
> -  AC_TRY_RUN([f(int i){*(double *)i=*(double *)(i+4);}
> -              int main(){char b[[12]];f(b);return 0;}],
> +  AC_TRY_RUN([
> +
> +int f(void* i){
> +       *(double *)i=*(double *)(i+4);
> +       return *(char*)i;
> +}
> +int main(){
> +       char b[[12]];
> +       b[[0]]=1;
> +       b[[1]]=2;
> +       b[[2]]=3;
> +       b[[3]]=4;
> +       b[[4]]=0;
> +       b[[5]]=0;
> +       b[[6]]=0;
> +       b[[7]]=0;
> +       b[[8]]=0;
> +       b[[9]]=0;
> +       b[[10]]=0;
> +       b[[11]]=0;
> +
> +       return f(b);
> +}
> +],
> +
>      ac_cv_double_align="yes", ac_cv_double_align="no"))
>  test "$ac_cv_double_align" = "no" && AC_DEFINE(DOUBLE_WORD_ALIGNMENT)])
>
>  AC_DEFUN([AC_C_DOUBLE_ORDER],
Reply | Threaded
Open this post in threaded view
|

Re: patch: fix double alignment detection on Solaris/SPARC with gcc-4.4.x

Ian Piumarta
 
Hi Dave,

On Jul 30, 2009, at 7:47 PM, David T. Lewis wrote:

> Ian,
> There is a patch for platforms/unix/config/acinclude.m4 at the end  
> of this
> email that addresses an issue for newer GCCs on SPARC. When you get  
> a chance,
> can you please update the acinclude.m4 and do a make configure?

I already did.

Cheers,
Ian

Reply | Threaded
Open this post in threaded view
|

Re: patch: fix double alignment detection on Solaris/SPARC with gcc-4.4.x

Andrew Gaylard
 
On Fri, Jul 31, 2009 at 5:05 AM, Ian Piumarta<[hidden email]> wrote:
> On Jul 30, 2009, at 7:47 PM, David T. Lewis wrote:
>
>> Ian,
>> There is a patch for platforms/unix/config/acinclude.m4 at the end of this
>> email that addresses an issue for newer GCCs on SPARC. When you get a
>> chance,
>> can you please update the acinclude.m4 and do a make configure?
>
> I already did.

Ian & David,

Thanks a lot for adding this patch to the tree so quickly.
I have several other patches which we rely on to fix various
issues, including:

- building on Solaris
- using sound on Solaris
- setting the X bitmap icon
- pasting from the clipboard
- changing the signal-handling code from signal() to sigaction()

The latter three are useful on both Solaris and Linux.

Would it be possible to get these patches into the tree too?

I've submitted them over the years, but there's been no feedback
whether or not they're the right fixes for these various problems.

Thanks,
Andrew
Reply | Threaded
Open this post in threaded view
|

Re: patch: fix double alignment detection on Solaris/SPARC with gcc-4.4.x

David T. Lewis
 
On Fri, Jul 31, 2009 at 08:14:34PM +0200, Andrew Gaylard wrote:

>  
> On Fri, Jul 31, 2009 at 5:05 AM, Ian Piumarta<[hidden email]> wrote:
> > On Jul 30, 2009, at 7:47 PM, David T. Lewis wrote:
> >
> >> Ian,
> >> There is a patch for platforms/unix/config/acinclude.m4 at the end of this
> >> email that addresses an issue for newer GCCs on SPARC. When you get a
> >> chance,
> >> can you please update the acinclude.m4 and do a make configure?
> >
> > I already did.
>
> Ian & David,
>
> Thanks a lot for adding this patch to the tree so quickly.
> I have several other patches which we rely on to fix various
> issues, including:
>
> - building on Solaris
> - using sound on Solaris
> - setting the X bitmap icon
> - pasting from the clipboard
> - changing the signal-handling code from signal() to sigaction()
>
> The latter three are useful on both Solaris and Linux.
>
> Would it be possible to get these patches into the tree too?
>
> I've submitted them over the years, but there's been no feedback
> whether or not they're the right fixes for these various problems.

Hi Andrew,

Ian can answer with respect to added the patches, but from my point
of view can I suggest that you add an issue on the Mantis bug tracking
system (http://bugs.squeak.org) and attach your patches there. Use
bug category "VM".

This won't guarantee anything getting done, but it does guarantee
that we will not lose your work. Patches posted to the list do have
a tendency to get lost in the ether sometimes.

Dave

Reply | Threaded
Open this post in threaded view
|

Re: patch: fix double alignment detection on Solaris/SPARC with gcc-4.4.x

Ian Piumarta
In reply to this post by Andrew Gaylard
 
Andrew,

> I have several other patches which we rely on to fix various issues

I have fix-to-build-solaris.diff and new-solaris-sound-driver.diff.  
If there's anything else, please send it my way.  (I spent yesterday  
afternoon trying to persuade Solaris 10 to install under kvm and  
failed.  I'll try again this weekend with VirtualBox.)

BTW: Subbu pointed out that the caps lock bug (where it's reported as  
shift pressed) crept back in during an olpc merge.  While fixing that  
I loaded Dave's latest VMM and rebuilt the interpreter.  SVN and the  
3.10-6 archives should have working closure support.  Binaries are  
there for Mac PPC+386, Ubuntu 8.04 386 and FreeBSD 7.2 386.

Cheers,
Ian