* [patch] libffi darwin x86-32bit, fix return_sc testcase.
@ 2008-01-02 21:47 Andreas Tobler
2008-01-03 9:52 ` Andrew Haley
0 siblings, 1 reply; 7+ messages in thread
From: Andreas Tobler @ 2008-01-02 21:47 UTC (permalink / raw)
To: GCC Patches, Java Patches, Eric Christopher; +Cc: Andrew Haley
[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]
Hello all,
first, a happy new year to all!
The attached patch brings libffi for darwin x86 (32-bit) back to zero
failures, at least what is covered by the test suite ;)
I implemented the fix according to Andrew's fix back in July 07 for
linux x86. Except that I had to treat small structs in special way.
I tested the patch on a Core 2 Duo with 10.5.1, including a libjava test
suite run. Nothing in regard of regression. But also no improvments for
TestClosureGC.jar. This is another iteration.
Is this ok for trunk?
It 'fixes' PR 32843 for darwin x86...
Thanks IA,
Regards,
Andreas
2008-01-02 Andreas Tobler <a.tobler@schweiz.org>
PR testsuite/32843
* src/x86/ffi.c (ffi_prep_cif_machdep): Add code for
signed/unsigned int8/16 for X86_DARWIN.
Updated copyright info.
Handle one and two byte structs with special cif->flags.
* src/x86/ffitarget.h: Add special types for one and two byte
structs.
Updated copyright info.
* src/x86/darwin.S (ffi_call_SYSV): Rewrite to use a jump table
like
sysv.S
Remove code to pop args from the stack after call.
Special-case signed/unsigned for int8/16, one and two byte structs.
(ffi_closure_raw_SYSV): Handle FFI_TYPE_UINT8,
FFI_TYPE_SINT8, FFI_TYPE_UINT16, FFI_TYPE_SINT16, FFI_TYPE_UINT32,
FFI_TYPE_SINT32.
Updated copyright info.
[-- Attachment #2: pr32843-darwin-1.diff --]
[-- Type: text/plain, Size: 8443 bytes --]
Index: src/x86/ffi.c
===================================================================
--- src/x86/ffi.c (revision 131248)
+++ src/x86/ffi.c (working copy)
@@ -3,9 +3,10 @@
Copyright (c) 2002 Ranjit Mathew
Copyright (c) 2002 Bo Thorsen
Copyright (c) 2002 Roger Sayle
-
- x86 Foreign Function Interface
+ Copyright (C) 2008 Free Software Foundation, Inc.
+ x86 Foreign Function Interface
+
Permission is hereby granted, free of charge, to any person obtaining
a copy of this software and associated documentation files (the
``Software''), to deal in the Software without restriction, including
@@ -121,10 +122,13 @@
case FFI_TYPE_VOID:
#ifdef X86
case FFI_TYPE_STRUCT:
+#else
+# if defined(X86) || defined(X86_DARWIN)
case FFI_TYPE_UINT8:
case FFI_TYPE_UINT16:
case FFI_TYPE_SINT8:
case FFI_TYPE_SINT16:
+# endif
#endif
case FFI_TYPE_SINT64:
@@ -142,11 +146,11 @@
case FFI_TYPE_STRUCT:
if (cif->rtype->size == 1)
{
- cif->flags = FFI_TYPE_SINT8; /* same as char size */
+ cif->flags = FFI_TYPE_SMALL_STRUCT_1B; /* same as char size */
}
else if (cif->rtype->size == 2)
{
- cif->flags = FFI_TYPE_SINT16; /* same as short size */
+ cif->flags = FFI_TYPE_SMALL_STRUCT_2B; /* same as short size */
}
else if (cif->rtype->size == 4)
{
Index: src/x86/ffitarget.h
===================================================================
--- src/x86/ffitarget.h (revision 131248)
+++ src/x86/ffitarget.h (working copy)
@@ -1,5 +1,7 @@
/* -----------------------------------------------------------------*-C-*-
ffitarget.h - Copyright (c) 1996-2003 Red Hat, Inc.
+ Copyright (C) 2008 Free Software Foundation, Inc.
+
Target configuration macros for x86 and x86-64.
Permission is hereby granted, free of charge, to any person obtaining
@@ -68,6 +70,8 @@
/* ---- Definitions for closures ----------------------------------------- */
#define FFI_CLOSURES 1
+#define FFI_TYPE_SMALL_STRUCT_1B (FFI_TYPE_LAST + 1)
+#define FFI_TYPE_SMALL_STRUCT_2B (FFI_TYPE_LAST + 2)
#if defined (X86_64) || (defined (__x86_64__) && defined (X86_DARWIN))
#define FFI_TRAMPOLINE_SIZE 24
Index: src/x86/darwin.S
===================================================================
--- src/x86/darwin.S (revision 131248)
+++ src/x86/darwin.S (working copy)
@@ -1,8 +1,9 @@
/* -----------------------------------------------------------------------
- sysv.S - Copyright (c) 1996, 1998, 2001, 2002, 2003, 2005 Red Hat, Inc.
-
- X86 Foreign Function Interface
+ darwin.S - Copyright (c) 1996, 1998, 2001, 2002, 2003, 2005 Red Hat, Inc.
+ Copyright (C) 2008 Free Software Foundation, Inc.
+ X86 Foreign Function Interface
+
Permission is hereby granted, free of charge, to any person obtaining
a copy of this software and associated documentation files (the
``Software''), to deal in the Software without restriction, including
@@ -60,16 +61,15 @@
call *28(%ebp)
- /* Remove the space we pushed for the args */
- movl 16(%ebp),%ecx
- addl %ecx,%esp
-
/* Load %ecx with the return type code */
movl 20(%ebp),%ecx
+ /* Protect %esi. We're going to pop it in the epilogue. */
+ pushl %esi
+
/* If the return value pointer is NULL, assume no return value. */
cmpl $0,24(%ebp)
- jne retint
+ jne 0f
/* Even if there is no space for the return value, we are
obliged to handle floating-point values. */
@@ -77,79 +77,102 @@
jne noretval
fstp %st(0)
- jmp epilogue
-
-retint:
- cmpl $FFI_TYPE_INT,%ecx
- jne retfloat
- /* Load %ecx with the pointer to storage for the return value */
- movl 24(%ebp),%ecx
- movl %eax,0(%ecx)
jmp epilogue
+0:
+ .align 4
+ call 1f
+.Lstore_table:
+ .long noretval-.Lstore_table /* FFI_TYPE_VOID */
+ .long retint-.Lstore_table /* FFI_TYPE_INT */
+ .long retfloat-.Lstore_table /* FFI_TYPE_FLOAT */
+ .long retdouble-.Lstore_table /* FFI_TYPE_DOUBLE */
+ .long retlongdouble-.Lstore_table /* FFI_TYPE_LONGDOUBLE */
+ .long retuint8-.Lstore_table /* FFI_TYPE_UINT8 */
+ .long retsint8-.Lstore_table /* FFI_TYPE_SINT8 */
+ .long retuint16-.Lstore_table /* FFI_TYPE_UINT16 */
+ .long retsint16-.Lstore_table /* FFI_TYPE_SINT16 */
+ .long retint-.Lstore_table /* FFI_TYPE_UINT32 */
+ .long retint-.Lstore_table /* FFI_TYPE_SINT32 */
+ .long retint64-.Lstore_table /* FFI_TYPE_UINT64 */
+ .long retint64-.Lstore_table /* FFI_TYPE_SINT64 */
+ .long retstruct-.Lstore_table /* FFI_TYPE_STRUCT */
+ .long retint-.Lstore_table /* FFI_TYPE_POINTER */
+ .long retstruct1b-.Lstore_table /* FFI_TYPE_SMALL_STRUCT_1B */
+ .long retstruct2b-.Lstore_table /* FFI_TYPE_SMALL_STRUCT_2B */
+1:
+ pop %esi
+ add (%esi, %ecx, 4), %esi
+ jmp *%esi
+ /* Sign/zero extend as appropriate. */
+retsint8:
+ movsbl %al, %eax
+ jmp retint
+
+retsint16:
+ movswl %ax, %eax
+ jmp retint
+
+retuint8:
+ movzbl %al, %eax
+ jmp retint
+
+retuint16:
+ movzwl %ax, %eax
+ jmp retint
+
retfloat:
- cmpl $FFI_TYPE_FLOAT,%ecx
- jne retdouble
/* Load %ecx with the pointer to storage for the return value */
- movl 24(%ebp),%ecx
+ movl 24(%ebp),%ecx
fstps (%ecx)
jmp epilogue
retdouble:
- cmpl $FFI_TYPE_DOUBLE,%ecx
- jne retlongdouble
/* Load %ecx with the pointer to storage for the return value */
- movl 24(%ebp),%ecx
+ movl 24(%ebp),%ecx
fstpl (%ecx)
jmp epilogue
retlongdouble:
- cmpl $FFI_TYPE_LONGDOUBLE,%ecx
- jne retint64
/* Load %ecx with the pointer to storage for the return value */
- movl 24(%ebp),%ecx
+ movl 24(%ebp),%ecx
fstpt (%ecx)
jmp epilogue
-
-retint64:
- cmpl $FFI_TYPE_SINT64,%ecx
- jne retstruct1b
+
+retint64:
/* Load %ecx with the pointer to storage for the return value */
- movl 24(%ebp),%ecx
+ movl 24(%ebp),%ecx
movl %eax,0(%ecx)
movl %edx,4(%ecx)
jmp epilogue
-
-retstruct1b:
- cmpl $FFI_TYPE_SINT8,%ecx
- jne retstruct2b
+
+retstruct1b:
/* Load %ecx with the pointer to storage for the return value */
- movl 24(%ebp),%ecx
+ movl 24(%ebp),%ecx
movb %al,0(%ecx)
jmp epilogue
-retstruct2b:
- cmpl $FFI_TYPE_SINT16,%ecx
- jne retstruct
+retstruct2b:
/* Load %ecx with the pointer to storage for the return value */
- movl 24(%ebp),%ecx
+ movl 24(%ebp),%ecx
movw %ax,0(%ecx)
jmp epilogue
+retint:
+ /* Load %ecx with the pointer to storage for the return value */
+ movl 24(%ebp),%ecx
+ movl %eax,0(%ecx)
+
retstruct:
- cmpl $FFI_TYPE_STRUCT,%ecx
- jne noretval
/* Nothing to do! */
- addl $4,%esp
- popl %ebp
- ret
noretval:
epilogue:
- addl $8,%esp
- movl %ebp,%esp
- popl %ebp
- ret
+ popl %esi
+ movl %ebp,%esp
+ popl %ebp
+ ret
+
.LFE1:
.ffi_call_SYSV_end:
@@ -177,7 +200,15 @@
movl -12(%ebp), %ecx
cmpl $FFI_TYPE_INT, %eax
je .Lcls_retint
- cmpl $FFI_TYPE_FLOAT, %eax
+
+ /* Handle FFI_TYPE_UINT8, FFI_TYPE_SINT8, FFI_TYPE_UINT16,
+ FFI_TYPE_SINT16, FFI_TYPE_UINT32, FFI_TYPE_SINT32. */
+ cmpl $FFI_TYPE_UINT64, %eax
+ jge 0f
+ cmpl $FFI_TYPE_UINT8, %eax
+ jge .Lcls_retint
+
+0: cmpl $FFI_TYPE_FLOAT, %eax
je .Lcls_retfloat
cmpl $FFI_TYPE_DOUBLE, %eax
je .Lcls_retdouble
@@ -185,10 +216,10 @@
je .Lcls_retldouble
cmpl $FFI_TYPE_SINT64, %eax
je .Lcls_retllong
- cmpl $FFI_TYPE_SINT8, %eax
- je .Lcls_retstruct1
- cmpl $FFI_TYPE_SINT16, %eax
- je .Lcls_retstruct2
+ cmpl $FFI_TYPE_SMALL_STRUCT_1B, %eax
+ je .Lcls_retstruct1b
+ cmpl $FFI_TYPE_SMALL_STRUCT_2B, %eax
+ je .Lcls_retstruct2b
cmpl $FFI_TYPE_STRUCT, %eax
je .Lcls_retstruct
.Lcls_epilogue:
@@ -211,10 +242,10 @@
movl (%ecx), %eax
movl 4(%ecx), %edx
jmp .Lcls_epilogue
-.Lcls_retstruct1:
+.Lcls_retstruct1b:
movsbl (%ecx), %eax
jmp .Lcls_epilogue
-.Lcls_retstruct2:
+.Lcls_retstruct2b:
movswl (%ecx), %eax
jmp .Lcls_epilogue
.Lcls_retstruct:
@@ -256,6 +287,14 @@
movl CIF_FLAGS_OFFSET(%esi), %eax /* rtype */
cmpl $FFI_TYPE_INT, %eax
je .Lrcls_retint
+
+ /* Handle FFI_TYPE_UINT8, FFI_TYPE_SINT8, FFI_TYPE_UINT16,
+ FFI_TYPE_SINT16, FFI_TYPE_UINT32, FFI_TYPE_SINT32. */
+ cmpl $FFI_TYPE_UINT64, %eax
+ jge 0f
+ cmpl $FFI_TYPE_UINT8, %eax
+ jge .Lrcls_retint
+0:
cmpl $FFI_TYPE_FLOAT, %eax
je .Lrcls_retfloat
cmpl $FFI_TYPE_DOUBLE, %eax
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] libffi darwin x86-32bit, fix return_sc testcase.
2008-01-02 21:47 [patch] libffi darwin x86-32bit, fix return_sc testcase Andreas Tobler
@ 2008-01-03 9:52 ` Andrew Haley
2008-01-03 10:05 ` Richard Guenther
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Haley @ 2008-01-03 9:52 UTC (permalink / raw)
To: Andreas Tobler
Cc: GCC Patches, Java Patches, Eric Christopher, Richard Guenther
Andreas Tobler writes:
> Hello all,
>
> first, a happy new year to all!
>
> The attached patch brings libffi for darwin x86 (32-bit) back to zero
> failures, at least what is covered by the test suite ;)
>
> I implemented the fix according to Andrew's fix back in July 07 for
> linux x86. Except that I had to treat small structs in special way.
I will again point out that this is merely a workaround for a gcc code
generation bug. Rather than fix the symptom, a libffi test failure,
we should fix the real problem: we are not generating ABI-compatible
code on x86.
This bug was introduced by
Author: rguenth
Date: Mon Jul 9 09:53:11 2007
New Revision: 126480
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32843
Andrew.
--
Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK
Registered in England and Wales No. 3798903
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] libffi darwin x86-32bit, fix return_sc testcase.
2008-01-03 9:52 ` Andrew Haley
@ 2008-01-03 10:05 ` Richard Guenther
2008-01-03 10:13 ` Andrew Haley
0 siblings, 1 reply; 7+ messages in thread
From: Richard Guenther @ 2008-01-03 10:05 UTC (permalink / raw)
To: Andrew Haley
Cc: Andreas Tobler, GCC Patches, Java Patches, Eric Christopher,
Michael Matz
On Thu, 3 Jan 2008, Andrew Haley wrote:
> Andreas Tobler writes:
> > Hello all,
> >
> > first, a happy new year to all!
> >
> > The attached patch brings libffi for darwin x86 (32-bit) back to zero
> > failures, at least what is covered by the test suite ;)
> >
> > I implemented the fix according to Andrew's fix back in July 07 for
> > linux x86. Except that I had to treat small structs in special way.
>
> I will again point out that this is merely a workaround for a gcc code
> generation bug. Rather than fix the symptom, a libffi test failure,
> we should fix the real problem: we are not generating ABI-compatible
> code on x86.
>
> This bug was introduced by
>
> Author: rguenth
> Date: Mon Jul 9 09:53:11 2007
> New Revision: 126480
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32843
We obviously disagree about what the ABI specifies. r126480 made the
C/C++ frontend behavior the same as other frontends behavior (for
example Fortran behavior, which I explicitly checked). As GCC supports
both promoting function return values or not in the backends and the
x86 backend explicitly does _not_ enable this promotion it agrees
with my reading of the ABI.
Richard.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] libffi darwin x86-32bit, fix return_sc testcase.
2008-01-03 10:05 ` Richard Guenther
@ 2008-01-03 10:13 ` Andrew Haley
2008-01-04 22:31 ` Andreas Tobler
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Haley @ 2008-01-03 10:13 UTC (permalink / raw)
To: Richard Guenther
Cc: Andreas Tobler, GCC Patches, Java Patches, Eric Christopher,
Michael Matz
Richard Guenther writes:
> We obviously disagree about what the ABI specifies. r126480 made
> the C/C++ frontend behavior the same as other frontends behavior
> (for example Fortran behavior, which I explicitly checked). As GCC
> supports both promoting function return values or not in the
> backends and the x86 backend explicitly does _not_ enable this
> promotion it agrees with my reading of the ABI.
But the ABI isn't in doubt: it is required to
> Functions pass all integer-valued arguments as words, expanding or
> padding signed or unsigned bytes and halfwords as needed'
and this surely applies to return values as well as arguments passed
to functions. It is utterly perverse to assume that return values are
treated differently from arguments passed to functions. So if the
back end isn't promoting return values then the back end is wrong.
Andrew.
--
Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK
Registered in England and Wales No. 3798903
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] libffi darwin x86-32bit, fix return_sc testcase.
2008-01-03 10:13 ` Andrew Haley
@ 2008-01-04 22:31 ` Andreas Tobler
2008-01-05 10:46 ` Andrew Haley
0 siblings, 1 reply; 7+ messages in thread
From: Andreas Tobler @ 2008-01-04 22:31 UTC (permalink / raw)
To: Andrew Haley, Richard Guenther
Cc: GCC Patches, Java Patches, Eric Christopher, Michael Matz
Andrew Haley wrote:
> Richard Guenther writes:
>
> > We obviously disagree about what the ABI specifies. r126480 made
> > the C/C++ frontend behavior the same as other frontends behavior
> > (for example Fortran behavior, which I explicitly checked). As GCC
> > supports both promoting function return values or not in the
> > backends and the x86 backend explicitly does _not_ enable this
> > promotion it agrees with my reading of the ABI.
>
> But the ABI isn't in doubt: it is required to
>
>> Functions pass all integer-valued arguments as words, expanding or
>> padding signed or unsigned bytes and halfwords as needed'
>
> and this surely applies to return values as well as arguments passed
> to functions. It is utterly perverse to assume that return values are
> treated differently from arguments passed to functions. So if the
> back end isn't promoting return values then the back end is wrong.
I'm not in the position to judge about this, I can only read the ABI and
I have to give my vote to aph, at least from what _I_ understand.
Maybe my understanding lacks some basic issues, in this case I'd like to
be advised to understand it right. But until then, I suppose this patch
is hold on ice, right?
This blocks my further work and I fear this issue might take some moons
to be resolved/clarified.
Who has the ability to clarify this issue? Do we need to expand the
to/cc list to the _right_ people? Who are the right people?
I'd like to have this clarified, even if I can't help that much in
clarifying....
Thanks,
Andreas
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] libffi darwin x86-32bit, fix return_sc testcase.
2008-01-04 22:31 ` Andreas Tobler
@ 2008-01-05 10:46 ` Andrew Haley
2008-01-06 12:17 ` Andreas Tobler
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Haley @ 2008-01-05 10:46 UTC (permalink / raw)
To: Andreas Tobler
Cc: Richard Guenther, GCC Patches, Java Patches, Eric Christopher,
Michael Matz
Andreas Tobler writes:
> Andrew Haley wrote:
> > Richard Guenther writes:
> >
> > > We obviously disagree about what the ABI specifies. r126480 made
> > > the C/C++ frontend behavior the same as other frontends behavior
> > > (for example Fortran behavior, which I explicitly checked). As GCC
> > > supports both promoting function return values or not in the
> > > backends and the x86 backend explicitly does _not_ enable this
> > > promotion it agrees with my reading of the ABI.
> >
> > But the ABI isn't in doubt: it is required to
> >
> >> Functions pass all integer-valued arguments as words, expanding or
> >> padding signed or unsigned bytes and halfwords as needed'
> >
> > and this surely applies to return values as well as arguments passed
> > to functions. It is utterly perverse to assume that return values are
> > treated differently from arguments passed to functions. So if the
> > back end isn't promoting return values then the back end is wrong.
>
> I'm not in the position to judge about this, I can only read the ABI and
> I have to give my vote to aph, at least from what _I_ understand.
> Maybe my understanding lacks some basic issues, in this case I'd like to
> be advised to understand it right. But until then, I suppose this patch
> is hold on ice, right?
> This blocks my further work and I fear this issue might take some moons
> to be resolved/clarified.
>
> Who has the ability to clarify this issue? Do we need to expand the
> to/cc list to the _right_ people? Who are the right people?
>
> I'd like to have this clarified, even if I can't help that much in
> clarifying....
The Darwin libffi patch is OK. There's no reason why it shouldn't go in.
I'll submit a patch for the x86 backend next week.
Andrew.
--
Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK
Registered in England and Wales No. 3798903
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch] libffi darwin x86-32bit, fix return_sc testcase.
2008-01-05 10:46 ` Andrew Haley
@ 2008-01-06 12:17 ` Andreas Tobler
0 siblings, 0 replies; 7+ messages in thread
From: Andreas Tobler @ 2008-01-06 12:17 UTC (permalink / raw)
Cc: GCC Patches, Java Patches
I committed the attached patch as it causes a failure on x86 linux.
Sorry.
This time tested on linux as well.
Andreas
2008-01-06 Andreas Tobler <a.tobler@schweiz.org>
* src/x86/ffi.c (ffi_prep_cif_machdep): Fix thinko.
Index: src/x86/ffi.c
===================================================================
--- src/x86/ffi.c (revision 131343)
+++ src/x86/ffi.c (working copy)
@@ -122,13 +122,12 @@
case FFI_TYPE_VOID:
#ifdef X86
case FFI_TYPE_STRUCT:
-#else
-# if defined(X86) || defined(X86_DARWIN)
+#endif
+#if defined(X86) || defined(X86_DARWIN)
case FFI_TYPE_UINT8:
case FFI_TYPE_UINT16:
case FFI_TYPE_SINT8:
case FFI_TYPE_SINT16:
-# endif
#endif
case FFI_TYPE_SINT64:
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-01-06 12:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-02 21:47 [patch] libffi darwin x86-32bit, fix return_sc testcase Andreas Tobler
2008-01-03 9:52 ` Andrew Haley
2008-01-03 10:05 ` Richard Guenther
2008-01-03 10:13 ` Andrew Haley
2008-01-04 22:31 ` Andreas Tobler
2008-01-05 10:46 ` Andrew Haley
2008-01-06 12:17 ` Andreas Tobler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).