Re: libffi fails to build on powerpc64-linux

Re: libffi fails to build on powerpc64-linux

[Dennis Clarke]

> Removing Kyle from the CC list, since his email address doesn't seem to
> be valid anymore and adding Andreas since his patch from 2009 mentioned
> below triggers the assert.
>
 I'll do some testing here on ppc970 and let you know what I see.

-- 
--
http://pgp.mit.edu:11371/pks/lookup?op=vindex&search=0x1D936C72FA35B44B
+-------------------------+-----------------------------------+
| Dennis Clarke           | Solaris and Linux and Open Source |
| dclarke@blastwave.org   | Respect for open standards.       |
+-------------------------+-----------------------------------+

Re: libffi fails to build on powerpc64-linux

[David Edelsohn]

> On Tue, 2012-03-06 at 09:34 -0600, Peter Bergner wrote:
> > On Mon, 2012-03-05 at 22:18 -0500, Anthony Green wrote:
> > > At this point, I'm hoping that Kyle Moffett, the author of this patch
> > > can have a look.  My guess is that either I mis-applied the patch, or
> > > he posted the wrong patch to apply.
> >
> > Agreed, it would be nice for Kyle to comment.  Looking at the bugzilla
> > above, he said:
> >
> > > This passes the testsuite on soft-floating-point PowerPC, and it builds
> > > and passes the testsuite on PowerPC e500 systems which cannot even
> > > assemble the regular floating-point instruction set.
> >
> > Both of these are non FP power systems, so I can see why he didn't
> > see these build errors, since the broken code is inside of
> > "#ifndef __NO_FPRS__" (double negative?).
>
> Looking at the bugzilla further, I noticed Kyle has a blog here that
> mentions he has moved to Google, so that explains why his email address
> was bouncing.  I'm also guessing he's no longer interested in PowerPC
> issues now that he's no longer at Boeing.
>
> That said, looking farther down that debian bugzilla, I noticed someone
> complained about his patch breaking the debian build.  He attached a
> patch:
>
>  http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=40;filename=libffi-powerpc-build-fix.patch;att=1;bug=644338
>
> If you ignore the debian specific changes and look at the src/powerpc/ffi.c
> change, you'll see it's the same as my patch, minus the hunks of my patch
> that are silencing warnings, so I think my patch is probably safe to take
> as is.  Therefore, I'm officially submitting the patch below to fix the
> build issues.
>
> That still leaves the 32-bit many.c testsuite breakage I am seeing, but I
> think that should be fixed in a separate patch since it is not related to
> these changes at all.
>
> Peter

Adding a different email address for Kyle to possibly entice him to
participate in this email thread.

- David

Re: libffi fails to build on powerpc64-linux

[Kyle Moffett]

On Thu, Mar 8, 2012 at 18:37, David Edelsohn <dje.gcc@gmail.com> wrote:
>> On Tue, 2012-03-06 at 09:34 -0600, Peter Bergner wrote:
>> > > This passes the testsuite on soft-floating-point PowerPC, and it builds
>> > > and passes the testsuite on PowerPC e500 systems which cannot even
>> > > assemble the regular floating-point instruction set.
>> >
>> > Both of these are non FP power systems, so I can see why he didn't
>> > see these build errors, since the broken code is inside of
>> > "#ifndef __NO_FPRS__" (double negative?).
>>
>> Looking at the bugzilla further, I noticed Kyle has a blog here that
>> mentions he has moved to Google, so that explains why his email address
>> was bouncing.  I'm also guessing he's no longer interested in PowerPC
>> issues now that he's no longer at Boeing.
>>
>> That said, looking farther down that debian bugzilla, I noticed someone
>> complained about his patch breaking the debian build.  He attached a
>> patch:
>>
>>  http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=40;filename=libffi-powerpc-build-fix.patch;att=1;bug=644338
>>
>> If you ignore the debian specific changes and look at the src/powerpc/ffi.c
>> change, you'll see it's the same as my patch, minus the hunks of my patch
>> that are silencing warnings, so I think my patch is probably safe to take
>> as is.  Therefore, I'm officially submitting the patch below to fix the
>> build issues.
>>
>> That still leaves the 32-bit many.c testsuite breakage I am seeing, but I
>> think that should be fixed in a separate patch since it is not related to
>> these changes at all.
>
> Adding a different email address for Kyle to possibly entice him to
> participate in this email thread.

Ah, hello there!  I'm afraid that I don't have any e500 systems to
test with anymore, but I will certainly say that the second fixup
patch I submitted to Debian is currently part of their official stack
of patches, and I built and tested it on powerpc, powerpc64, and
powerpcspe after having that problem with the first one.  I'm very
sorry for not following up and forwarding the fixed patch upstream
properly.

I believe Jeff Runningen is the current eXMeritus point-of-contact for
the Debian e500 stuff, although he probably hasn't had to touch libffi
yet and most likely doesn't have the time unless it's terminally
broken. :-D  I've CCed him anyways in the hope that he will have this
background in the event that he runs into problems in the future.

My apologies for the trouble, good luck!

Cheers,
Kyle Moffett

Re: libffi fails to build on powerpc64-linux

[Dennis Clarke]

> (let me preface this by apologizing for the build breakage)
>
> On Mon, Mar 5, 2012 at 7:28 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
>> Taking my best swag at where the soft_double_prep label should be
>> (comment said it should be handled like UINT64), I tried the following
>> patch which allows everything to build without warnings and seems to
>> pass the testsuite:
>>
>>
>>                === libffi Summary ===
>>
>> # of expected passes            1659
>> # of unsupported tests          55
>
> Those results look fine, however, the soft_double_prep label was
> specifically removed by this patch...
>
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=powerpc-ffi-softfloat.patch;att=1;bug=644338
>
> ...which was designed to enable support for soft-float ppc targets,
> among other things.   I see now that the original patch didn't remove
> all references to soft_double_prep.
>
> At this point, I'm hoping that Kyle Moffett, the author of this patch
> can have a look.  My guess is that either I mis-applied the patch, or
> he posted the wrong patch to apply.

I am no where near my powermac g5 but as soon as I can I'll climb back
in and see what I can get done with libffi. The entire objective of my
efforts was to get a clean bootstrap of GCC 4.6.2 for ppc970 and then
to work towards GCC 4.7.0RC1 etc.  Thus this is a bit of a road block
that must be cleared on ppc.

Dennis



-- 
--
http://pgp.mit.edu:11371/pks/lookup?op=vindex&search=0x1D936C72FA35B44B
+-------------------------+-----------------------------------+
| Dennis Clarke           | Solaris and Linux and Open Source |
| dclarke@blastwave.org   | Respect for open standards.       |
+-------------------------+-----------------------------------+

Re: libffi fails to build on powerpc64-linux

[Peter Bergner]

On Mon, 2012-03-05 at 23:20 -0500, Dennis Clarke wrote:
> I am no where near my powermac g5 but as soon as I can I'll climb back
> in and see what I can get done with libffi. The entire objective of my
> efforts was to get a clean bootstrap of GCC 4.6.2 for ppc970 and then
> to work towards GCC 4.7.0RC1 etc.  Thus this is a bit of a road block
> that must be cleared on ppc.

I just built mainline libffi on my G5 installed with Fedora 16 which has
GCC 4.6.2 installed.  Building went fine (using my latest patch from my
other email) and so did the testsuite run.

I'll mention that Fedora 16's GCC defaults to producing 64-bit binaries.
Are you trying to build libffi on a system where GCC defaults to producing
32-bit binaries and you're trying to produce a 64-bit libffi?

Peter

Re: libffi fails to build on powerpc64-linux

[Dennis Clarke]

> After the latest libffi merge into GCC, GCC no longer builds on
> powerpc64-linux due to the following errors:

I ran into a bucket of issues with libffi also :

  http://sourceware.org/ml/libffi-discuss/2012/msg00048.html

May or may not be related.

dc



-- 
--
http://pgp.mit.edu:11371/pks/lookup?op=vindex&search=0x1D936C72FA35B44B
+-------------------------+-----------------------------------+
| Dennis Clarke           | Solaris and Linux and Open Source |
| dclarke@blastwave.org   | Respect for open standards.       |
+-------------------------+-----------------------------------+

Re: libffi fails to build on powerpc64-linux

[Peter Bergner]

On Mon, 2012-03-05 at 18:05 -0500, Dennis Clarke wrote:
> > After the latest libffi merge into GCC, GCC no longer builds on
> > powerpc64-linux due to the following errors:
> 
> I ran into a bucket of issues with libffi also :
> 
>   http://sourceware.org/ml/libffi-discuss/2012/msg00048.html
> 
> May or may not be related.

Not related, since I'm not even getting past the build, let alone
firing off the testsuite.  I'll let you know what happens when
I get that far. :)

That said, adding your --enable-debug configure option, I see a few
other changes that should be made to silence some warnings about
casting a pointer to an integer of a different size (ie, smaller):


@@ -155,9 +156,9 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned
*const stack)
   next_arg.u = stack + 2;
 
   /* Check that everything starts aligned properly.  */
-  FFI_ASSERT (((unsigned) (char *) stack & 0xF) == 0);
-  FFI_ASSERT (((unsigned) copy_space.c & 0xF) == 0);
-  FFI_ASSERT (((unsigned) stacktop.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) (char *) stack & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) copy_space.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) stacktop.c & 0xF) == 0);
   FFI_ASSERT ((bytes & 0xF) == 0);
   FFI_ASSERT (copy_space.c >= next_arg.c);


Peter

Re: libffi fails to build on powerpc64-linux

[Peter Bergner]

Taking my best swag at where the soft_double_prep label should be
(comment said it should be handled like UINT64), I tried the following
patch which allows everything to build without warnings and seems to
pass the testsuite:


		=== libffi Summary ===

# of expected passes		1659
# of unsupported tests		55


Comments?


Peter


        * src/powerpc/ffi.c (ffi_prep_args_SYSV): Declare double_tmp.
        Cast pointers to unsigned long.
        Declare soft_double_prep label.
        (ffi_call): Silence possibly undefined warning.
        (ffi_closure_helper_SYSV): Declare variable type.


diff --git a/src/powerpc/ffi.c b/src/powerpc/ffi.c
index 1920c91..627b4cb 100644
--- a/src/powerpc/ffi.c
+++ b/src/powerpc/ffi.c
@@ -146,6 +146,7 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
   gpr_base.u = stacktop.u - ASM_NEEDS_REGISTERS - NUM_GPR_ARG_REGISTERS;
   intarg_count = 0;
 #ifndef __NO_FPRS__
+  double double_tmp;
   fpr_base.d = gpr_base.d - NUM_FPR_ARG_REGISTERS;
   fparg_count = 0;
   copy_space.c = ((flags & FLAG_FP_ARGUMENTS) ? fpr_base.c : gpr_base.c);
@@ -155,9 +156,9 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
   next_arg.u = stack + 2;
 
   /* Check that everything starts aligned properly.  */
-  FFI_ASSERT (((unsigned) (char *) stack & 0xF) == 0);
-  FFI_ASSERT (((unsigned) copy_space.c & 0xF) == 0);
-  FFI_ASSERT (((unsigned) stacktop.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) (char *) stack & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) copy_space.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) stacktop.c & 0xF) == 0);
   FFI_ASSERT ((bytes & 0xF) == 0);
   FFI_ASSERT (copy_space.c >= next_arg.c);
 
@@ -293,6 +294,9 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
 
 	case FFI_TYPE_UINT64:
 	case FFI_TYPE_SINT64:
+
+	soft_double_prep:
+
 	  if (intarg_count == NUM_GPR_ARG_REGISTERS-1)
 	    intarg_count++;
 	  if (intarg_count >= NUM_GPR_ARG_REGISTERS)
@@ -925,7 +929,7 @@ ffi_call(ffi_cif *cif, void (*fn)(void), void *rvalue, void **avalue)
    */
   unsigned int smst_buffer[2];
   extended_cif ecif;
-  unsigned int rsize;
+  unsigned int rsize = 0;
 
   ecif.cif = cif;
   ecif.avalue = avalue;
@@ -1132,7 +1136,7 @@ ffi_closure_helper_SYSV (ffi_closure *closure, void *rvalue,
 
 	  if (nf < 8)
 	    {
-	      temp = pfr->d;
+	      double temp = pfr->d;
 	      pfr->f = (float) temp;
 	      avalue[i] = pfr;
 	      nf++;

Re: libffi fails to build on powerpc64-linux

[Anthony Green]

(let me preface this by apologizing for the build breakage)

On Mon, Mar 5, 2012 at 7:28 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> Taking my best swag at where the soft_double_prep label should be
> (comment said it should be handled like UINT64), I tried the following
> patch which allows everything to build without warnings and seems to
> pass the testsuite:
>
>
>                === libffi Summary ===
>
> # of expected passes            1659
> # of unsupported tests          55

Those results look fine, however, the soft_double_prep label was
specifically removed by this patch...

http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=powerpc-ffi-softfloat.patch;att=1;bug=644338

...which was designed to enable support for soft-float ppc targets,
among other things.   I see now that the original patch didn't remove
all references to soft_double_prep.

At this point, I'm hoping that Kyle Moffett, the author of this patch
can have a look.  My guess is that either I mis-applied the patch, or
he posted the wrong patch to apply.

Thanks!

AG




>
>
> Comments?
>
>
> Peter
>
>
>        * src/powerpc/ffi.c (ffi_prep_args_SYSV): Declare double_tmp.
>        Cast pointers to unsigned long.
>        Declare soft_double_prep label.
>        (ffi_call): Silence possibly undefined warning.
>        (ffi_closure_helper_SYSV): Declare variable type.
>
>
> diff --git a/src/powerpc/ffi.c b/src/powerpc/ffi.c
> index 1920c91..627b4cb 100644
> --- a/src/powerpc/ffi.c
> +++ b/src/powerpc/ffi.c
> @@ -146,6 +146,7 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
>   gpr_base.u = stacktop.u - ASM_NEEDS_REGISTERS - NUM_GPR_ARG_REGISTERS;
>   intarg_count = 0;
>  #ifndef __NO_FPRS__
> +  double double_tmp;
>   fpr_base.d = gpr_base.d - NUM_FPR_ARG_REGISTERS;
>   fparg_count = 0;
>   copy_space.c = ((flags & FLAG_FP_ARGUMENTS) ? fpr_base.c : gpr_base.c);
> @@ -155,9 +156,9 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
>   next_arg.u = stack + 2;
>
>   /* Check that everything starts aligned properly.  */
> -  FFI_ASSERT (((unsigned) (char *) stack & 0xF) == 0);
> -  FFI_ASSERT (((unsigned) copy_space.c & 0xF) == 0);
> -  FFI_ASSERT (((unsigned) stacktop.c & 0xF) == 0);
> +  FFI_ASSERT (((unsigned long) (char *) stack & 0xF) == 0);
> +  FFI_ASSERT (((unsigned long) copy_space.c & 0xF) == 0);
> +  FFI_ASSERT (((unsigned long) stacktop.c & 0xF) == 0);
>   FFI_ASSERT ((bytes & 0xF) == 0);
>   FFI_ASSERT (copy_space.c >= next_arg.c);
>
> @@ -293,6 +294,9 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
>
>        case FFI_TYPE_UINT64:
>        case FFI_TYPE_SINT64:
> +
> +       soft_double_prep:
> +
>          if (intarg_count == NUM_GPR_ARG_REGISTERS-1)
>            intarg_count++;
>          if (intarg_count >= NUM_GPR_ARG_REGISTERS)
> @@ -925,7 +929,7 @@ ffi_call(ffi_cif *cif, void (*fn)(void), void *rvalue, void **avalue)
>    */
>   unsigned int smst_buffer[2];
>   extended_cif ecif;
> -  unsigned int rsize;
> +  unsigned int rsize = 0;
>
>   ecif.cif = cif;
>   ecif.avalue = avalue;
> @@ -1132,7 +1136,7 @@ ffi_closure_helper_SYSV (ffi_closure *closure, void *rvalue,
>
>          if (nf < 8)
>            {
> -             temp = pfr->d;
> +             double temp = pfr->d;
>              pfr->f = (float) temp;
>              avalue[i] = pfr;
>              nf++;
>
>

Re: libffi fails to build on powerpc64-linux

[Peter Bergner]

On Mon, 2012-03-05 at 22:18 -0500, Anthony Green wrote:
> (let me preface this by apologizing for the build breakage)

No worries.  I'm not here to point fingers, just trying to help fix the
breakage.


> On Mon, Mar 5, 2012 at 7:28 PM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> > Taking my best swag at where the soft_double_prep label should be
> > (comment said it should be handled like UINT64), I tried the following
> > patch which allows everything to build without warnings and seems to
> > pass the testsuite:
> >
> >
> >                === libffi Summary ===
> >
> > # of expected passes            1659
> > # of unsupported tests          55
> 
> Those results look fine, however, the soft_double_prep label was
> specifically removed by this patch...
> 
> http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=powerpc-ffi-softfloat.patch;att=1;bug=644338
> 
> ...which was designed to enable support for soft-float ppc targets,
> among other things.   I see now that the original patch didn't remove
> all references to soft_double_prep.

Ahh, looking at Kyle's patch, his patch removes the goto soft_float_prep
goto and label, so maybe he just forgot to delete the soft_double_prep
goto as well.  Nuking that with my patch builds and the make check
results are the same as before.


> At this point, I'm hoping that Kyle Moffett, the author of this patch
> can have a look.  My guess is that either I mis-applied the patch, or
> he posted the wrong patch to apply.

Agreed, it would be nice for Kyle to comment.  Looking at the bugzilla
above, he said:

> This passes the testsuite on soft-floating-point PowerPC, and it builds
> and passes the testsuite on PowerPC e500 systems which cannot even
> assemble the regular floating-point instruction set.

Both of these are non FP power systems, so I can see why he didn't
see these build errors, since the broken code is inside of
"#ifndef __NO_FPRS__" (double negative?).


My updated patch removing the soft_double_prep goto is below.
This was tested on my POWER7 and ppc970 systems (gcc's on both
systems default to creating 64-bit binaries).

Doing a build on one of my old POWER5 systems with a gcc that
defaults to producing 32-bit binaries, I'm seeing the following
testsuite failures:

Running /home/bergner/src/libffi-src/testsuite/libffi.call/call.exp ...
FAIL: libffi.call/many.c -O0 -W -Wall execution test
FAIL: libffi.call/many.c -O2 execution test
FAIL: libffi.call/many.c -O3 execution test
FAIL: libffi.call/many.c -Os execution test
FAIL: libffi.call/many.c -O2 -fomit-frame-pointer execution test
Running /home/bergner/src/libffi-src/testsuite/libffi.special/special.exp ...

		=== libffi Summary ===

# of expected passes		1654
# of unexpected failures	5
# of unsupported tests		55

I'll debug those to try and find out what is happening.

Peter



	* src/powerpc/ffi.c (ffi_prep_args_SYSV): Declare double_tmp.
	Silence casting pointer to integer of different size warning.
	(ffi_call): Silence possibly undefined warning.
	(ffi_closure_helper_SYSV): Declare variable type.

diff --git a/src/powerpc/ffi.c b/src/powerpc/ffi.c
index 1920c91..baca694 100644
--- a/src/powerpc/ffi.c
+++ b/src/powerpc/ffi.c
@@ -146,6 +146,7 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
   gpr_base.u = stacktop.u - ASM_NEEDS_REGISTERS - NUM_GPR_ARG_REGISTERS;
   intarg_count = 0;
 #ifndef __NO_FPRS__
+  double double_tmp;
   fpr_base.d = gpr_base.d - NUM_FPR_ARG_REGISTERS;
   fparg_count = 0;
   copy_space.c = ((flags & FLAG_FP_ARGUMENTS) ? fpr_base.c : gpr_base.c);
@@ -155,9 +156,9 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
   next_arg.u = stack + 2;
 
   /* Check that everything starts aligned properly.  */
-  FFI_ASSERT (((unsigned) (char *) stack & 0xF) == 0);
-  FFI_ASSERT (((unsigned) copy_space.c & 0xF) == 0);
-  FFI_ASSERT (((unsigned) stacktop.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) (char *) stack & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) copy_space.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) stacktop.c & 0xF) == 0);
   FFI_ASSERT ((bytes & 0xF) == 0);
   FFI_ASSERT (copy_space.c >= next_arg.c);
 
@@ -211,8 +212,6 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
 
 	case FFI_TYPE_DOUBLE:
 	  /* With FFI_LINUX_SOFT_FLOAT doubles are handled like UINT64.  */
-	  if (ecif->cif->abi == FFI_LINUX_SOFT_FLOAT)
-	    goto soft_double_prep;
 	  double_tmp = **p_argv.d;
 
 	  if (fparg_count >= NUM_FPR_ARG_REGISTERS)
@@ -925,7 +924,7 @@ ffi_call(ffi_cif *cif, void (*fn)(void), void *rvalue, void **avalue)
    */
   unsigned int smst_buffer[2];
   extended_cif ecif;
-  unsigned int rsize;
+  unsigned int rsize = 0;
 
   ecif.cif = cif;
   ecif.avalue = avalue;
@@ -1132,7 +1131,7 @@ ffi_closure_helper_SYSV (ffi_closure *closure, void *rvalue,
 
 	  if (nf < 8)
 	    {
-	      temp = pfr->d;
+	      double temp = pfr->d;
 	      pfr->f = (float) temp;
 	      avalue[i] = pfr;
 	      nf++;

Re: libffi fails to build on powerpc64-linux

[Peter Bergner]

On Tue, 2012-03-06 at 09:34 -0600, Peter Bergner wrote:
On Mon, 2012-03-05 at 22:18 -0500, Anthony Green wrote:
>> At this point, I'm hoping that Kyle Moffett, the author of this patch
>> can have a look.  My guess is that either I mis-applied the patch, or
>> he posted the wrong patch to apply.
>
> Agreed, it would be nice for Kyle to comment.  Looking at the bugzilla
> above, he said:

[<00>] XMail bounce: Rcpt=[kyle.d.moffett@boeing.com];Error=[550 5.0.0 <kyle.d.moffett@boeing.com>... User unknown]

Hmm, just a temporary mail issue or has Kyle's email address changed?


Peter

Re: libffi fails to build on powerpc64-linux

[Peter Bergner]

Removing Kyle from the CC list, since his email address doesn't seem to
be valid anymore and adding Andreas since his patch from 2009 mentioned
below triggers the assert.



On Tue, 2012-03-06 at 09:34 -0600, Peter Bergner wrote:


> Doing a build on one of my old POWER5 systems with a gcc that
> defaults to producing 32-bit binaries, I'm seeing the following
> testsuite failures:
> 
> Running /home/bergner/src/libffi-src/testsuite/libffi.call/call.exp ...
> FAIL: libffi.call/many.c -O0 -W -Wall execution test
> FAIL: libffi.call/many.c -O2 execution test
> FAIL: libffi.call/many.c -O3 execution test
> FAIL: libffi.call/many.c -Os execution test
> FAIL: libffi.call/many.c -O2 -fomit-frame-pointer execution test
> Running /home/bergner/src/libffi-src/testsuite/libffi.special/special.exp ...
> 
> 		=== libffi Summary ===
> 
> # of expected passes		1654
> # of unexpected failures	5
> # of unsupported tests		55
> 
> I'll debug those to try and find out what is happening.

The many.c test case tests the calling of a function with 13 float
formal arguments (and zero integer arguments).  This test fails due
to us hitting an assert in ffi_prep_args_SYSV():

  FFI_ASSERT (flags & FLAG_4_GPR_ARGUMENTS || intarg_count <= 4);

In this case flags does not have the FLAG_4_GPR_ARGUMENTS bit set because
ffi_prep_cif_machdep() computed that there were zero integer arguments.
However, ffi_prep_args_SYSV() somehow thinks there are 5 integer arguments
due to this code:

        case FFI_TYPE_FLOAT:
          /* With FFI_LINUX_SOFT_FLOAT floats are handled like UINT32.  */
          double_tmp = **p_argv.f;
          if (fparg_count >= NUM_FPR_ARG_REGISTERS)
            {
              *next_arg.f = (float) double_tmp;
              next_arg.u += 1;
              intarg_count++;
            }
          else
            ...

At first, I thought this was an obvious counting error, but looking at
when that code was added, Andreas said it was to fix a ppc32 bug:

  http://sourceware.org/ml/libffi-discuss/2009/msg00360.html

Does anyone remember exactly how/why that fixed the bug?  Looking at
the use of intarg_count in ffi_prep_args_SYSV(), it looks like it is
used solely for determining the alignment of arguments that are passed
on the stack and not specifically for actually counting the actual
number of integer register arguments which the assert is assuming.
Should we just remove that assert (I tried that and the test case
now passes) or should we try and be more strict about when it is
incremented and computing the stack alignment for args differently?


Peter

Re: libffi fails to build on powerpc64-linux

[Peter Bergner]

On Tue, 2012-03-06 at 09:34 -0600, Peter Bergner wrote:
> On Mon, 2012-03-05 at 22:18 -0500, Anthony Green wrote:
> > At this point, I'm hoping that Kyle Moffett, the author of this patch
> > can have a look.  My guess is that either I mis-applied the patch, or
> > he posted the wrong patch to apply.
> 
> Agreed, it would be nice for Kyle to comment.  Looking at the bugzilla
> above, he said:
> 
> > This passes the testsuite on soft-floating-point PowerPC, and it builds
> > and passes the testsuite on PowerPC e500 systems which cannot even
> > assemble the regular floating-point instruction set.
> 
> Both of these are non FP power systems, so I can see why he didn't
> see these build errors, since the broken code is inside of
> "#ifndef __NO_FPRS__" (double negative?).

Looking at the bugzilla further, I noticed Kyle has a blog here that
mentions he has moved to Google, so that explains why his email address
was bouncing.  I'm also guessing he's no longer interested in PowerPC
issues now that he's no longer at Boeing.

That said, looking farther down that debian bugzilla, I noticed someone
complained about his patch breaking the debian build.  He attached a
patch:

  http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=40;filename=libffi-powerpc-build-fix.patch;att=1;bug=644338


If you ignore the debian specific changes and look at the src/powerpc/ffi.c
change, you'll see it's the same as my patch, minus the hunks of my patch
that are silencing warnings, so I think my patch is probably safe to take
as is.  Therefore, I'm officially submitting the patch below to fix the
build issues.

That still leaves the 32-bit many.c testsuite breakage I am seeing, but I
think that should be fixed in a separate patch since it is not related to
these changes at all.

Peter


	* src/powerpc/ffi.c (ffi_prep_args_SYSV): Declare double_tmp.
	Silence casting pointer to integer of different size warning.
	(ffi_call): Silence possibly undefined warning.
	(ffi_closure_helper_SYSV): Declare variable type.

diff --git a/src/powerpc/ffi.c b/src/powerpc/ffi.c
index 1920c91..baca694 100644
--- a/src/powerpc/ffi.c
+++ b/src/powerpc/ffi.c
@@ -146,6 +146,7 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
   gpr_base.u = stacktop.u - ASM_NEEDS_REGISTERS - NUM_GPR_ARG_REGISTERS;
   intarg_count = 0;
 #ifndef __NO_FPRS__
+  double double_tmp;
   fpr_base.d = gpr_base.d - NUM_FPR_ARG_REGISTERS;
   fparg_count = 0;
   copy_space.c = ((flags & FLAG_FP_ARGUMENTS) ? fpr_base.c : gpr_base.c);
@@ -155,9 +156,9 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
   next_arg.u = stack + 2;
 
   /* Check that everything starts aligned properly.  */
-  FFI_ASSERT (((unsigned) (char *) stack & 0xF) == 0);
-  FFI_ASSERT (((unsigned) copy_space.c & 0xF) == 0);
-  FFI_ASSERT (((unsigned) stacktop.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) (char *) stack & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) copy_space.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) stacktop.c & 0xF) == 0);
   FFI_ASSERT ((bytes & 0xF) == 0);
   FFI_ASSERT (copy_space.c >= next_arg.c);
 
@@ -211,8 +212,6 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
 
 	case FFI_TYPE_DOUBLE:
 	  /* With FFI_LINUX_SOFT_FLOAT doubles are handled like UINT64.  */
-	  if (ecif->cif->abi == FFI_LINUX_SOFT_FLOAT)
-	    goto soft_double_prep;
 	  double_tmp = **p_argv.d;
 
 	  if (fparg_count >= NUM_FPR_ARG_REGISTERS)
@@ -925,7 +924,7 @@ ffi_call(ffi_cif *cif, void (*fn)(void), void *rvalue, void **avalue)
    */
   unsigned int smst_buffer[2];
   extended_cif ecif;
-  unsigned int rsize;
+  unsigned int rsize = 0;
 
   ecif.cif = cif;
   ecif.avalue = avalue;
@@ -1132,7 +1131,7 @@ ffi_closure_helper_SYSV (ffi_closure *closure, void *rvalue,
 
 	  if (nf < 8)
 	    {
-	      temp = pfr->d;
+	      double temp = pfr->d;
 	      pfr->f = (float) temp;
 	      avalue[i] = pfr;
 	      nf++;

Re: libffi fails to build on powerpc64-linux

[Peter Bergner]

On Thu, 2012-03-08 at 17:14 -0600, Peter Bergner wrote:
> 	* src/powerpc/ffi.c (ffi_prep_args_SYSV): Declare double_tmp.
> 	Silence casting pointer to integer of different size warning.
> 	(ffi_call): Silence possibly undefined warning.
> 	(ffi_closure_helper_SYSV): Declare variable type.

Bah, incomplete ChangeLog entry.  Here's the updated ChangeLog
along with the patch again.  Sorry.

Peter


	* src/powerpc/ffi.c (ffi_prep_args_SYSV): Declare double_tmp.
	Silence casting pointer to integer of different size warning.
	Delete goto to previously deleted label.
	(ffi_call): Silence possibly undefined warning.
	(ffi_closure_helper_SYSV): Declare variable type.

diff --git a/src/powerpc/ffi.c b/src/powerpc/ffi.c
index 1920c91..baca694 100644
--- a/src/powerpc/ffi.c
+++ b/src/powerpc/ffi.c
@@ -146,6 +146,7 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
   gpr_base.u = stacktop.u - ASM_NEEDS_REGISTERS - NUM_GPR_ARG_REGISTERS;
   intarg_count = 0;
 #ifndef __NO_FPRS__
+  double double_tmp;
   fpr_base.d = gpr_base.d - NUM_FPR_ARG_REGISTERS;
   fparg_count = 0;
   copy_space.c = ((flags & FLAG_FP_ARGUMENTS) ? fpr_base.c : gpr_base.c);
@@ -155,9 +156,9 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
   next_arg.u = stack + 2;
 
   /* Check that everything starts aligned properly.  */
-  FFI_ASSERT (((unsigned) (char *) stack & 0xF) == 0);
-  FFI_ASSERT (((unsigned) copy_space.c & 0xF) == 0);
-  FFI_ASSERT (((unsigned) stacktop.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) (char *) stack & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) copy_space.c & 0xF) == 0);
+  FFI_ASSERT (((unsigned long) stacktop.c & 0xF) == 0);
   FFI_ASSERT ((bytes & 0xF) == 0);
   FFI_ASSERT (copy_space.c >= next_arg.c);
 
@@ -211,8 +212,6 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
 
 	case FFI_TYPE_DOUBLE:
 	  /* With FFI_LINUX_SOFT_FLOAT doubles are handled like UINT64.  */
-	  if (ecif->cif->abi == FFI_LINUX_SOFT_FLOAT)
-	    goto soft_double_prep;
 	  double_tmp = **p_argv.d;
 
 	  if (fparg_count >= NUM_FPR_ARG_REGISTERS)
@@ -925,7 +924,7 @@ ffi_call(ffi_cif *cif, void (*fn)(void), void *rvalue, void **avalue)
    */
   unsigned int smst_buffer[2];
   extended_cif ecif;
-  unsigned int rsize;
+  unsigned int rsize = 0;
 
   ecif.cif = cif;
   ecif.avalue = avalue;
@@ -1132,7 +1131,7 @@ ffi_closure_helper_SYSV (ffi_closure *closure, void *rvalue,
 
 	  if (nf < 8)
 	    {
-	      temp = pfr->d;
+	      double temp = pfr->d;
 	      pfr->f = (float) temp;
 	      avalue[i] = pfr;
 	      nf++;

libffi fails to build on powerpc64-linux

[Peter Bergner]

After the latest libffi merge into GCC, GCC no longer builds on
powerpc64-linux due to the following errors:

/home/bergner/gcc/gcc-mainline-testsuite-base/libffi/src/powerpc/ffi.c: In function ‘ffi_prep_args_SYSV’:
/home/bergner/gcc/gcc-mainline-testsuite-base/libffi/src/powerpc/ffi.c:199:4: error: ‘double_tmp’ undeclared (first use in this function)
/home/bergner/gcc/gcc-mainline-testsuite-base/libffi/src/powerpc/ffi.c:199:4: note: each undeclared identifier is reported only once for each function it appears in
/home/bergner/gcc/gcc-mainline-testsuite-base/libffi/src/powerpc/ffi.c:215:6: error: label ‘soft_double_prep’ used but not defined
/home/bergner/gcc/gcc-mainline-testsuite-base/libffi/src/powerpc/ffi.c: In function ‘ffi_closure_helper_SYSV’:
/home/bergner/gcc/gcc-mainline-testsuite-base/libffi/src/powerpc/ffi.c:1135:8: error: ‘temp’ undeclared (first use in this function)


Building upstream libffi code directly, it fails exactly the same way.
The "double_tmp" and "temp" errors are easily fixed with the following
obvious patch:

diff --git a/src/powerpc/ffi.c b/src/powerpc/ffi.c
index 1920c91..191b9dc 100644
--- a/src/powerpc/ffi.c
+++ b/src/powerpc/ffi.c
@@ -146,6 +146,7 @@ ffi_prep_args_SYSV (extended_cif *ecif, unsigned *const stack)
   gpr_base.u = stacktop.u - ASM_NEEDS_REGISTERS - NUM_GPR_ARG_REGISTERS;
   intarg_count = 0;
 #ifndef __NO_FPRS__
+  double double_tmp;
   fpr_base.d = gpr_base.d - NUM_FPR_ARG_REGISTERS;
   fparg_count = 0;
   copy_space.c = ((flags & FLAG_FP_ARGUMENTS) ? fpr_base.c : gpr_base.c);
@@ -1132,7 +1133,7 @@ ffi_closure_helper_SYSV (ffi_closure *closure, void *rvalue,
 
          if (nf < 8)
            {
-             temp = pfr->d;
+             double temp = pfr->d;
              pfr->f = (float) temp;
              avalue[i] = pfr;
              nf++;

But I'm not confident enough to place the missing ‘soft_double_prep’ label.
Can someone more familiar with this code patch that and then remerge the
updated code into GCC?  I'm willing to help build and test any patch
someone comes up with.

As an FYI, I'll mention I opened a GCC bugzilla to track this on the
GCC side of things:

    http://gcc.gnu.org/PR52497


Peter