[PATCH, go] Passing Complex64 and Complex128 values via reflect.Call (using libffi) introduces ABI mismatch

[Uros Bizjak]

Hello!

Due to the fact that libFFI does not handle C99 _Complex arguments
correctly [1], libgo passes Complex64 and Complex128 arguments via a
temporary structure. However, passing parts of complex number in a
structure is not the same as passing true C99 _Complex value, so this
workaround introduces ABI mismatch between caller and callee. This
mismatch results in wrong passed values of complex types.

Fortunately all x86 ABIs tolerate this mismatch, but other targets
(i.e. alpha) don't have this privilege.

Attached patch disables passing of C99 _Complex arguments via FFI on
all targets, other than x86 (to be on the safe side w.r.t. other
targets). Hopefully, someday libffi will be extended to handle
_Complex arguments in correct way.

Patch was tested on x86_64-pc-linux-gnu {,-m32} and alphaev68-pc-linux-gnu.

[1] http://sourceware.org/ml/libffi-discuss/2007/msg00010.html

Uros.

[Ian Lance Taylor]

On Fri, Mar 1, 2013 at 4:57 AM, Uros Bizjak <ubi...@gmail.com> wrote:
>
> Due to the fact that libFFI does not handle C99 _Complex arguments
> correctly [1], libgo passes Complex64 and Complex128 arguments via a
> temporary structure. However, passing parts of complex number in a
> structure is not the same as passing true C99 _Complex value, so this
> workaround introduces ABI mismatch between caller and callee. This
> mismatch results in wrong passed values of complex types.
>
> Fortunately all x86 ABIs tolerate this mismatch, but other targets
> (i.e. alpha) don't have this privilege.

Is there a PR open against libffi?

Do we have any idea which targets pass complex in a manner different
than a struct of two float/doubles? Your patch assumes that only x86
targets work, but I would expect that many targets work that way.

Ian

[Uros Bizjak]

On Fri, Mar 1, 2013 at 3:43 PM, Ian Lance Taylor <ia...@google.com> wrote:
> On Fri, Mar 1, 2013 at 4:57 AM, Uros Bizjak <ubi...@gmail.com> wrote:
>>
>> Due to the fact that libFFI does not handle C99 _Complex arguments
>> correctly [1], libgo passes Complex64 and Complex128 arguments via a
>> temporary structure. However, passing parts of complex number in a
>> structure is not the same as passing true C99 _Complex value, so this
>> workaround introduces ABI mismatch between caller and callee. This
>> mismatch results in wrong passed values of complex types.
>>
>> Fortunately all x86 ABIs tolerate this mismatch, but other targets
>> (i.e. alpha) don't have this privilege.
>
> Is there a PR open against libffi?

Not that I know of, but I can open one if requested.

> Do we have any idea which targets pass complex in a manner different
> than a struct of two float/doubles? Your patch assumes that only x86
> targets work, but I would expect that many targets work that way.

Maybe a test could be added to reflect package that calls foreign
function with _Complex arguments/return value? The function should do
some basic arithmetic on complex values and the test could check if it
returns expected values. This test would reliably catch affected
targets.

Uros.

[Uros Bizjak]

On Fri, Mar 1, 2013 at 3:50 PM, Uros Bizjak <ubi...@gmail.com> wrote:

>>> Due to the fact that libFFI does not handle C99 _Complex arguments
>>> correctly [1], libgo passes Complex64 and Complex128 arguments via a
>>> temporary structure. However, passing parts of complex number in a
>>> structure is not the same as passing true C99 _Complex value, so this
>>> workaround introduces ABI mismatch between caller and callee. This
>>> mismatch results in wrong passed values of complex types.
>>>
>>> Fortunately all x86 ABIs tolerate this mismatch, but other targets
>>> (i.e. alpha) don't have this privilege.
>>
>> Is there a PR open against libffi?
>
> Not that I know of, but I can open one if requested.

FYI, python is also interested [1] in this enhancement.

[1] http://bugs.python.org/issue16899

Uros.

[Uros Bizjak]

On Fri, Mar 1, 2013 at 3:43 PM, Ian Lance Taylor <ia...@google.com> wrote:

$ grep -R "define TARGET_SPLIT_COMPLEX_ARG" config
config/rs6000/rs6000.c:#define TARGET_SPLIT_COMPLEX_ARG
hook_bool_const_tree_true
config/xtensa/xtensa.c:#define TARGET_SPLIT_COMPLEX_ARG
hook_bool_const_tree_true
config/alpha/alpha.c:#define TARGET_SPLIT_COMPLEX_ARG alpha_split_complex_arg

We can probably define Complex64 type as "struct { float re, im; }"
and Complex128 as "struct { double re, im; }" for these targets. What
do you think about this approach?

Uros.

[Ian Lance Taylor]

The odd thing is that those are exactly the targets I would expect to
work with the current code. I don't see why those should be the
targets that fail.

Right now the reflect.Call implementation is passing Go complex64 and
complex128 types as though they were implemented as struct { float32;
float32 } and struct { float64; float64 } respectively. The code
assumes that a function that takes a complex64 argument can be called
as though it took a struct argument instead. Apparently that does not
work on Alpha. So the case that fails is when the calling convention
for a complex number differs from the calling convention for passing a
struct.

In the Alpha calling convention, how is a complex number passed? How
is a struct of two floats/doubles passed?

I think your suggestion of implementing complex as a struct would be
somewhat painful to implement, because it would require generating
complicated code for all complex arithmetic.

I'm not strongly opposed to your original patch, I just think it
overreaches in assuming that only x86 works. What if we flip it
around and assume that only Alpha falis?

Ian

[Uros Bizjak]

On Fri, Mar 1, 2013 at 7:26 PM, Ian Lance Taylor <ia...@google.com> wrote:

> Right now the reflect.Call implementation is passing Go complex64 and
> complex128 types as though they were implemented as struct { float32;
> float32 } and struct { float64; float64 } respectively. The code
> assumes that a function that takes a complex64 argument can be called
> as though it took a struct argument instead. Apparently that does not
> work on Alpha. So the case that fails is when the calling convention
> for a complex number differs from the calling convention for passing a
> struct.
>
> In the Alpha calling convention, how is a complex number passed? How
> is a struct of two floats/doubles passed?

Looking at the following testcase, they are passed in integer
registers, and returned in memory.

--cut here--
typedef struct
{
float re, im;
} cfloat_t;

cfloat_t testf (cfloat_t a, cfloat_t b)
{
cfloat_t r;

r.re = a.re + b.re;
r.im = a.im + b.im;
return r;
}

_Complex float _testf (_Complex float a, _Complex float b)
{
return a + b;
}
--cut here--

testf:
.frame $30,16,$26,0
lda $30,-16($30)
.prologue 0
mov $16,$0
stq $17,0($30)
stq $18,8($30)
lds $f10,12($30)
lds $f11,4($30)
lds $f12,0($30)
bis $31,$31,$31
adds $f11,$f10,$f11
lds $f10,8($30)
adds $f12,$f10,$f10
sts $f11,4($16)
sts $f10,0($16)
bis $31,$31,$31
lda $30,16($30)
ret $31,($26),1

_testf:
.frame $30,0,$26,0
.prologue 0
adds $f17,$f19,$f1
adds $f16,$f18,$f0
ret $31,($26),1

[the double assembly is practically the same]

> I think your suggestion of implementing complex as a struct would be
> somewhat painful to implement, because it would require generating
> complicated code for all complex arithmetic.
>
> I'm not strongly opposed to your original patch, I just think it
> overreaches in assuming that only x86 works. What if we flip it
> around and assume that only Alpha falis?

No problem for me, the attached patch was re-tested with libgo tests
on alphaev68-pc-linux-gnu and x86_64-pc-linux-gnu {,-m32} without
errors.

Uros.

[Ian Lance Taylor]

On Fri, Mar 1, 2013 at 10:50 AM, Uros Bizjak <ubi...@gmail.com> wrote:
>
> No problem for me, the attached patch was re-tested with libgo tests
> on alphaev68-pc-linux-gnu and x86_64-pc-linux-gnu {,-m32} without
> errors.

Committed with slightly different error messages, like so.

Thanks.

Ian