Message ID | 20141212120630.GA32026@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On 12/12/2014 04:06 AM, Dominik Vogt wrote: > I'm not sure I've posted the missing patch anywhere yet, so it's > attached to this message. At the moment it enables > FFI_TYPE_COMPLEX only for s390[x], but eventually this should be > used unconditionally. Thanks for that. I'd been meaning to get around to that. I'll change the test to use FFI_TARGET_HAS_COMPLEX_TYPE and apply it to my branch. > (This still leaves the dynamic linking issue if we do not use > libffi for reflection calls with x86* and s390[x]. Is the plan to > remove the platform specific abi code for the few platforms that > have it? I see no way to make them work with the static chain > patch anyway.) Well, the x86 paths were updated to work with the static chain, but indeed that required assembly rather than cheating and using C as you did. But removing all of that was always my goal. Indeed, my branch now has a patch to remove all of the target-specific code. Tested only on x86_64 so far, but I plan to test i686 today. r~
On Fri, Dec 12, 2014 at 10:14:21AM -0800, Richard Henderson wrote: > On 12/12/2014 04:06 AM, Dominik Vogt wrote: > > I'm not sure I've posted the missing patch anywhere yet, so it's > > attached to this message. At the moment it enables > > FFI_TYPE_COMPLEX only for s390[x], but eventually this should be > > used unconditionally. > > Thanks for that. I'd been meaning to get around to that. I'll change the test > to use FFI_TARGET_HAS_COMPLEX_TYPE and apply it to my branch. Good. I'm not sure whether it's a good idea to expose FFI_TARGET_HAS_COMPLEX_TYPE as part of the libffi interface though. It was meant as a temporary thing to be removed once all platforms supported by libffi have implemented complex support. A while ago I've posted a patch to change the macro's name to begin with an underscore to make that clearer. > > (This still leaves the dynamic linking issue if we do not use > > libffi for reflection calls with x86* and s390[x]. Is the plan to > > remove the platform specific abi code for the few platforms that > > have it? I see no way to make them work with the static chain > > patch anyway.) > > Well, the x86 paths were updated to work with the static chain, but indeed that > required assembly rather than cheating and using C as you did. > > But removing all of that was always my goal. Indeed, my branch now has a patch > to remove all of the target-specific code. Fine with that. I wouldn't have written the s390 specific Abi code in Go if libffi had been an option back then. Ciao Dominik ^_^ ^_^
On 12/15/2014 03:42 AM, Dominik Vogt wrote: > On Fri, Dec 12, 2014 at 10:14:21AM -0800, Richard Henderson wrote: >> On 12/12/2014 04:06 AM, Dominik Vogt wrote: >>> I'm not sure I've posted the missing patch anywhere yet, so it's >>> attached to this message. At the moment it enables >>> FFI_TYPE_COMPLEX only for s390[x], but eventually this should be >>> used unconditionally. >> >> Thanks for that. I'd been meaning to get around to that. I'll change the test >> to use FFI_TARGET_HAS_COMPLEX_TYPE and apply it to my branch. > > Good. I'm not sure whether it's a good idea to expose > FFI_TARGET_HAS_COMPLEX_TYPE as part of the libffi interface > though. It was meant as a temporary thing to be removed once all > platforms supported by libffi have implemented complex support. A > while ago I've posted a patch to change the macro's name to begin > with an underscore to make that clearer. It's our copy of libffi -- I think we can assume any internals we like. Similarly, when I finish writing the bits that allow libffi to handle empty structures, I don't plan to conditionalize libgo, I simply plan to assume it works. r~
From 84235d9e7ba8a55dea182adc4007bfab6a35fb1f Mon Sep 17 00:00:00 2001 From: Dominik Vogt <vogt@de.ibm.com> Date: Wed, 29 Oct 2014 09:08:01 +0100 Subject: [PATCH] libgo: Enable complex number support from libffi. --- libgo/runtime/go-ffi.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/libgo/runtime/go-ffi.c b/libgo/runtime/go-ffi.c index 21879b9..42462d0 100644 --- a/libgo/runtime/go-ffi.c +++ b/libgo/runtime/go-ffi.c @@ -150,11 +150,26 @@ go_complex_to_ffi (ffi_type *float_type) ffi_type *ret; ret = (ffi_type *) __go_alloc (sizeof (ffi_type)); + /* Use libffi with complex type support for targets that have it. This should + be the case for all targets eventually, so the #else branch should then be + removed. */ +#if defined (__s390__) && defined (FFI_TYPE_COMPLEX) + ret->type = FFI_TYPE_COMPLEX; + ret->size = 2 * float_type->size; + ret->alignment = float_type->alignment; + ret->elements = (ffi_type **) __go_alloc (2 * sizeof (ffi_type *)); + ret->elements[0] = float_type; + ret->elements[1] = NULL; +#else + /* Warning: This works only on platforms that define C _Complex types like + structures in their Abi. */ ret->type = FFI_TYPE_STRUCT; ret->elements = (ffi_type **) __go_alloc (3 * sizeof (ffi_type *)); ret->elements[0] = float_type; ret->elements[1] = float_type; ret->elements[2] = NULL; +#endif + return ret; } @@ -184,6 +199,9 @@ go_type_to_ffi (const struct __go_type_descriptor *descriptor) #ifdef __alpha__ runtime_throw("the libffi library does not support Complex64 type with " "reflect.Call or runtime.SetFinalizer"); +#elif defined(__s390__) && !defined(FFI_TYPE_COMPLEX) + runtime_throw("the libffi library does not support Complex64 type with " + "reflect.Call or runtime.SetFinalizer"); #else if (sizeof (float) == 4) return go_complex_to_ffi (&ffi_type_float); @@ -193,6 +211,9 @@ go_type_to_ffi (const struct __go_type_descriptor *descriptor) #ifdef __alpha__ runtime_throw("the libffi library does not support Complex128 type with " "reflect.Call or runtime.SetFinalizer"); +#elif defined(__s390__) && !defined(FFI_TYPE_COMPLEX) + runtime_throw("the libffi library does not support Complex128 type with " + "reflect.Call or runtime.SetFinalizer"); #else if (sizeof (double) == 8) return go_complex_to_ffi (&ffi_type_double); -- 1.8.4.2