diff mbox

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

Message ID CAFULd4ZTsCPc-ji6QqHfgMGCtX_giv84eGE-5NEt3X4jXOdrmA@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak March 1, 2013, 6:50 p.m. UTC 
On Fri, Mar 1, 2013 at 7:26 PM, Ian Lance Taylor <iant@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.

Comments

Ian Lance Taylor March 1, 2013, 7:27 p.m. UTC | #1 
On Fri, Mar 1, 2013 at 10:50 AM, Uros Bizjak <ubizjak@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
diff mbox

Patch

Index: go/testing/quick/quick_test.go
===================================================================
--- go/testing/quick/quick_test.go	(revision 196386)
+++ go/testing/quick/quick_test.go	(working copy)
@@ -7,6 +7,7 @@  package quick
 import (
 	"math/rand"
 	"reflect"
+	"runtime"
 	"testing"
 )
 
@@ -72,8 +73,10 @@  func TestCheckEqual(t *testing.T) {
 	reportError("fBool", CheckEqual(fBool, fBool, nil), t)
 	reportError("fFloat32", CheckEqual(fFloat32, fFloat32, nil), t)
 	reportError("fFloat64", CheckEqual(fFloat64, fFloat64, nil), t)
-	reportError("fComplex64", CheckEqual(fComplex64, fComplex64, nil), t)
-	reportError("fComplex128", CheckEqual(fComplex128, fComplex128, nil), t)
+	if runtime.GOARCH != "alpha" {
+		reportError("fComplex64", CheckEqual(fComplex64, fComplex64, nil), t)
+		reportError("fComplex128", CheckEqual(fComplex128, fComplex128, nil), t)
+	}
 	reportError("fInt16", CheckEqual(fInt16, fInt16, nil), t)
 	reportError("fInt32", CheckEqual(fInt32, fInt32, nil), t)
 	reportError("fInt64", CheckEqual(fInt64, fInt64, nil), t)
Index: runtime/go-reflect-call.c
===================================================================
--- runtime/go-reflect-call.c	(revision 196386)
+++ runtime/go-reflect-call.c	(working copy)
@@ -30,7 +30,7 @@  static ffi_type *go_struct_to_ffi (const struct __
 static ffi_type *go_string_to_ffi (void) __attribute__ ((no_split_stack));
 static ffi_type *go_interface_to_ffi (void) __attribute__ ((no_split_stack));
 static ffi_type *go_complex_to_ffi (ffi_type *)
-  __attribute__ ((no_split_stack));
+  __attribute__ ((no_split_stack,unused));
 static ffi_type *go_type_to_ffi (const struct __go_type_descriptor *)
   __attribute__ ((no_split_stack));
 static ffi_type *go_func_return_ffi (const struct __go_func_type *)
@@ -185,13 +185,23 @@  go_type_to_ffi (const struct __go_type_descriptor
 	return &ffi_type_double;
       abort ();
     case GO_COMPLEX64:
+#ifdef __alpha__
+      runtime_throw("the ABI does not support Complex64 type with "
+		    "reflect.Call or runtime.SetFinalizer");
+#else
       if (sizeof (float) == 4)
 	return go_complex_to_ffi (&ffi_type_float);
       abort ();
+#endif
     case GO_COMPLEX128:
+#ifdef __alpha__
+      runtime_throw("the ABI does not support Complex128 type with "
+		    "reflect.Call or runtime.SetFinalizer");
+#else
       if (sizeof (double) == 8)
 	return go_complex_to_ffi (&ffi_type_double);
       abort ();
+#endif
     case GO_INT16:
       return &ffi_type_sint16;
     case GO_INT32: