Re: [gofrontend-dev] Re: Go patch committed: Fix error reporting for invalid builtin calls

2015-08-03 Thread Michael Hudson-Doyle
Now I get

../../../gcc/libgo/runtime/mprof.goc: In function ‘runtime_Stack’:
../../../gcc/libgo/runtime/mprof.goc:437:19: error: ‘enablegc’ may be
used uninitialized in this function [-Werror=maybe-uninitialized]
   mstats.enablegc = enablegc;
   ^
../../../gcc/libgo/runtime/mprof.goc:406:7: note: ‘enablegc’ was declared here
  bool enablegc;
   ^

Am I doing something wrong?

Cheers,
mwh

On 4 August 2015 at 05:55, Ian Lance Taylor  wrote:
> On Mon, Aug 3, 2015 at 2:10 AM, Andreas Schwab  wrote:
>
>> ../../../libgo/runtime/mprof.goc: In function 'runtime_Stack':
>> ../../../libgo/runtime/mprof.goc:408:5: error: calling 
>> '__builtin_frame_address' with a nonzero argument is unsafe 
>> [-Werror=frame-address]
>>   sp = runtime_getcallersp(&b);
>
> Fixed by this patch by Chris Manghane.  The call was not actually
> necessary.  Bootstrapped and ran Go testsuite on
> x86_64-unknown-linux-gnu.  Committed to mainline.  This fixes PR
> 67101.
>
> Ian
>
> --
> You received this message because you are subscribed to the Google Groups 
> "gofrontend-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to gofrontend-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.


Re: [gofrontend-dev] [PATCH 1/2, libgo] Add reflection support to gccgo for ppc64, ppc64le in gcc 4.9

2015-01-07 Thread Michael Hudson-Doyle
Ian Lance Taylor  writes:

> On Wed, Jan 7, 2015 at 9:26 AM, Lynn A. Boger
>  wrote:
>>
>> In libgo/go/reflect/makefunc.go, calls to MakeFunc, makeMethodValue and
>> makeValueMethod will panic if called when GOARCH is ppc64 or ppc64le.
>
> Right, I'm just saying that almost no code actually does that.  I just
> tried a web search and found no uses other than examples of how to use
> it.  I'm sure there are a few, but not many.

There is a somewhat hidden one in Docker:

https://github.com/docker/docker/blob/master/api/client/cli.go#L64

(it is also possible to patch docker to do this differently, of course).

Cheers,
mwh


pgpdSphHFvTaL.pgp
Description: PGP signature


Re: [gofrontend-dev] [PATCH 4/4] Gccgo port to s390[x] -- part II

2014-11-09 Thread Michael Hudson-Doyle
Ian Taylor  writes:

> I don't know what's up with the complex number change.  In general the
> Go compiler and libraries go to some effort to produce the same
> answers on all platforms.  We need to understand why we get different
> answers on s390 (you may understand the differences, but I don't).

Oh, I know this one.  I've even filed yet another bug about it:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59714

I assume s390 has a fused multiply add instruction?  It's because
libgcc's implementation of complex division is written in a way such
that gcc compiles an expression like a*b-c*d as fma(a,b,-c*d) and even if
a==c and b==d the latter expression doesn't return 0 unless things are
in power of 2 ratios with one another.

> I won't change the tests without a clear understanding of why we are
> changing them.

I think the *real* fix is for libgcc to use ""Kahan's compensated
algorithm for 2 by 2 determinants"[1] to compute a*b-c*d when fma is
available.

Cheers,
mwh

[1] That's something like this:

// This implements what is sometimes called "Kahan's compensated algorithm 
for
// 2 by 2 determinants".  It returns a*b + c*d to a high degree of precision
// but depends on a fused-multiply add operation that rounds once.
//
// The obvious algorithm has problems when a*b and c*d nearly cancel, but 
the
// trick is the calculation of 'e': "a*b = w + e" is exact when the operands
// are considered as real numbers.  So if c*d nearly cancels out w, e 
restores
// the result to accuracy.
double
Kahan(double a, double b, double c, double d)
{
  double w, e, f;
  w = b * a;
  e = fma(b, a, -w);
  f = fma(d, c, w);
  return f + e;
}

Algorithms like this is why the fma operation was introduced in the
first place!


pgpEbbVCSQqC8.pgp
Description: PGP signature


Re: [gofrontend-dev] gccgo and syscall.SysProcAttr.Cloneflags

2014-09-06 Thread Michael Hudson-Doyle
Ian Lance Taylor  writes:

> On Mon, Sep 1, 2014 at 4:18 AM, Michael Hudson-Doyle
>  wrote:
>>
>> It's late for me and I don't have a proper test case but it seems to me
>> that while gccgo's syscall lets you set Cloneflags on its SysProcAttr,
>> but doesn't actually *do* anything with the flags.  Am I missing
>> something?
>
> You aren't missing anything.  I made an error in an libgo merge last
> year.  This patch fixes the problem.  

Hi, I can confirm that docker's libcontainer works much better with this
patch!  Thanks for the quick fix.

Cheers,
mwh

> Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
> Committed to mainline and 4.9 branch.
>
> Ian
>
> diff -r da369647d0ec libgo/go/syscall/exec_linux.go
> --- a/libgo/go/syscall/exec_linux.go Fri Sep 05 07:42:57 2014 -0700
> +++ b/libgo/go/syscall/exec_linux.go Fri Sep 05 08:05:22 2014 -0700
> @@ -43,7 +43,7 @@
>   // Declare all variables at top in case any
>   // declarations require heap allocation (e.g., err1).
>   var (
> - r1 Pid_t
> + r1 uintptr
>   err1   Errno
>   nextfd int
>   i  int
> @@ -65,7 +65,7 @@
>   // About to call fork.
>   // No more allocation or calls of non-assembly functions.
>   runtime_BeforeFork()
> - r1, err1 = raw_fork()
> + r1, _, err1 = RawSyscall6(SYS_CLONE,
> uintptr(SIGCHLD)|sys.Cloneflags, 0, 0, 0, 0, 0)
>   if err1 != 0 {
>   runtime_AfterFork()
>   return 0, err1


pgphVdlMFWmUt.pgp
Description: PGP signature


Re: [gofrontend-dev] libgo patch committed: Fix madvise on systems with page size != 4096

2014-04-27 Thread Michael Hudson-Doyle
"'Ian Lance Taylor ' via gofrontend-dev"
 writes:

> This patch from Anton Blanchard fixes libgo to adjust to the system page
> size when calling madvise.  Bootstrapped and ran Go testsuite on
> x86_64-unknown-linux-gnu.  Committed to mainline and 4.9 branch.

Hi, I think this patch will make my Canonical colleagues very happy (and
me when I get around to building a 64k page arm64 kernel...).  It looks
to me like this is also a problem in the gc runtime library though --
should the patch be sent there too?  (Apologies if it already has and I
missed it, I did look though).

Cheers,
mwh

> Ian
>
> -- 
> You received this message because you are subscribed to the Google Groups 
> "gofrontend-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to gofrontend-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
> diff -r 3a53301d24d7 libgo/runtime/mheap.c
> --- a/libgo/runtime/mheap.c   Tue Apr 22 16:43:35 2014 -0700
> +++ b/libgo/runtime/mheap.c   Thu Apr 24 21:18:35 2014 -0700
> @@ -387,7 +387,7 @@
>  static uintptr
>  scavengelist(MSpan *list, uint64 now, uint64 limit)
>  {
> - uintptr released, sumreleased;
> + uintptr released, sumreleased, start, end, pagesize;
>   MSpan *s;
>  
>   if(runtime_MSpanList_IsEmpty(list))
> @@ -400,7 +400,17 @@
>   mstats.heap_released += released;
>   sumreleased += released;
>   s->npreleased = s->npages;
> - runtime_SysUnused((void*)(s->start << PageShift), 
> s->npages << PageShift);
> +
> + start = s->start << PageShift;
> + end = start + (s->npages << PageShift);
> +
> + // Round start up and end down to ensure we
> + // are acting on entire pages.
> + pagesize = getpagesize();
> + start = ROUND(start, pagesize);
> + end &= ~(pagesize - 1);
> + if(end > start)
> + runtime_SysUnused((void*)start, end - start);
>   }
>   }
>   return sumreleased;


Re: libgo patch committed: Compile math library with -ffp-contract=off

2014-03-16 Thread Michael Hudson-Doyle
Michael Hudson-Doyle  writes:

> Ian Lance Taylor  writes:
>
>> On Thu, Mar 13, 2014 at 6:27 PM, Michael Hudson-Doyle
>>  wrote:
>>> Ian Lance Taylor  writes:
>>>
>>>> The bug report http://golang.org/issue/7074 shows that math.Log2(1)
>>>> produces the wrong result on Aarch64, because the Go math package is
>>>> compiled to use a fused multiply-add instruction.  This patch to the
>>>> libgo configure script will use -ffp-contract=off when compiling the
>>>> math package on processors other than x86.  Bootstrapped and ran Go
>>>> testsuite on x86_64-unknown-linux-gnu, not that that tests much.
>>>> Committed to mainline.
>>>
>>> Thanks for this!  If you are willing to go into battle enough to argue
>>> that libgcc should also be compiled with -ffp-contract=off (I did not
>>> have the stomach for that fight) then we'll be down to 1 check-go
>>> failure on aarch64 (which is peano -- due to the absence of
>>> split/copyable stacks and should probably xfail).
>>
>> Hmmm, what is it that fails with libgcc?  Is there a bug report for
>> it?
>
> https://code.google.com/p/go/issues/detail?id=7066
>
> and then
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59714
>
> I wanted to propose a version using "Kahan's algorithm for the
> determinant" as described in
>
> http://hal-ens-lyon.archives-ouvertes.fr/docs/00/78/57/86/PDF/Jeannerod_Louvet_Muller_final.pdf
>
> but I haven't gotten around to it...

I got bored / distracted and hacked up this:

diff --git a/libgo/runtime/go-cdiv.c b/libgo/runtime/go-cdiv.c
index 0a81e45..b96576a 100644
--- a/libgo/runtime/go-cdiv.c
+++ b/libgo/runtime/go-cdiv.c
@@ -13,6 +13,8 @@
the the whole number is Inf, but an operation involving NaN ought
to result in NaN, not Inf.  */
 
+#include 
+
 __complex float
 __go_complex64_div (__complex float a, __complex float b)
 {
@@ -29,6 +31,48 @@ __go_complex64_div (__complex float a, __complex float b)
   return a / b;
 }
 
+#ifdef FP_FAST_FMA
+
+// This implements what is sometimes called "Kahan's compensated algorithm for
+// 2 by 2 determinants".  It returns a*b + c*d to a high degree of precision
+// but depends on a fused-multiply add operation that rounds once.
+//
+// The obvious algorithm has problems when a*b and c*d nearly cancel, but the
+// trick is the calculation of 'e': "a*b = w + e" is exact when the operands
+// are considered as real numbers.  So if c*d nearly cancels out w, e restores
+// the result to accuracy.
+double
+Kahan(double a, double b, double c, double d)
+{
+  double w, e, f;
+  w = b * a;
+  e = fma(b, a, -w);
+  f = fma(d, c, w);
+  return f + e;
+}
+
+__complex double
+__go_complex128_div (__complex double a, __complex double b)
+{
+  double r, i, denom;
+  if (__builtin_expect (b == 0+0i, 0))
+{
+  if (!__builtin_isinf (__real__ a)
+ && !__builtin_isinf (__imag__ a)
+ && (__builtin_isnan (__real__ a) || __builtin_isnan (__imag__ a)))
+   {
+ /* Pass "1" to nan to match math/bits.go.  */
+ return __builtin_nan("1") + __builtin_nan("1")*1i;
+   }
+}
+  r = Kahan(__real__ a, __real__ b, __imag__ a, __imag__ b);
+  i = Kahan(__imag__ a, __real__ b, - __real__ a, __imag__ b);
+  denom = (__real__ b)*(__real__ b) + (__imag__ b)*(__imag__ b);
+  return r/denom + i*1.0i/denom;
+}
+
+#else
+
 __complex double
 __go_complex128_div (__complex double a, __complex double b)
 {
@@ -44,3 +88,5 @@ __go_complex128_div (__complex double a, __complex double b)
 }
   return a / b;
 }
+
+#endif

it would be better to do this in libgcc of course but I think that's
awkward because libgcc can't link to libm and so on...  It's probably a
little slower than the libgcc version (although this is straight line
code) but I don't really care about that :-)

Cheers,
mwh


Re: libgo patch committed: Compile math library with -ffp-contract=off

2014-03-13 Thread Michael Hudson-Doyle
Ian Lance Taylor  writes:

> On Thu, Mar 13, 2014 at 6:27 PM, Michael Hudson-Doyle
>  wrote:
>> Ian Lance Taylor  writes:
>>
>>> The bug report http://golang.org/issue/7074 shows that math.Log2(1)
>>> produces the wrong result on Aarch64, because the Go math package is
>>> compiled to use a fused multiply-add instruction.  This patch to the
>>> libgo configure script will use -ffp-contract=off when compiling the
>>> math package on processors other than x86.  Bootstrapped and ran Go
>>> testsuite on x86_64-unknown-linux-gnu, not that that tests much.
>>> Committed to mainline.
>>
>> Thanks for this!  If you are willing to go into battle enough to argue
>> that libgcc should also be compiled with -ffp-contract=off (I did not
>> have the stomach for that fight) then we'll be down to 1 check-go
>> failure on aarch64 (which is peano -- due to the absence of
>> split/copyable stacks and should probably xfail).
>
> Hmmm, what is it that fails with libgcc?  Is there a bug report for
> it?

https://code.google.com/p/go/issues/detail?id=7066

and then

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59714

I wanted to propose a version using "Kahan's algorithm for the
determinant" as described in

http://hal-ens-lyon.archives-ouvertes.fr/docs/00/78/57/86/PDF/Jeannerod_Louvet_Muller_final.pdf

but I haven't gotten around to it...

Cheers,
mwh

> I agree that peano is likely to fail without split stacks.
>
> Ian


Re: libgo patch committed: Compile math library with -ffp-contract=off

2014-03-13 Thread Michael Hudson-Doyle
Ian Lance Taylor  writes:

> The bug report http://golang.org/issue/7074 shows that math.Log2(1)
> produces the wrong result on Aarch64, because the Go math package is
> compiled to use a fused multiply-add instruction.  This patch to the
> libgo configure script will use -ffp-contract=off when compiling the
> math package on processors other than x86.  Bootstrapped and ran Go
> testsuite on x86_64-unknown-linux-gnu, not that that tests much.
> Committed to mainline.

Thanks for this!  If you are willing to go into battle enough to argue
that libgcc should also be compiled with -ffp-contract=off (I did not
have the stomach for that fight) then we'll be down to 1 check-go
failure on aarch64 (which is peano -- due to the absence of
split/copyable stacks and should probably xfail).

Cheers,
mwh

> Ian
>
> diff -r 76dbb6f77e3d libgo/configure.ac
> --- a/libgo/configure.ac  Tue Mar 11 12:53:06 2014 -0700
> +++ b/libgo/configure.ac  Tue Mar 11 21:26:35 2014 -0700
> @@ -620,6 +620,8 @@
>  MATH_FLAG=
>  if test "$libgo_cv_c_fancymath" = yes; then
>MATH_FLAG="-mfancy-math-387 -funsafe-math-optimizations"
> +else
> +  MATH_FLAG="-ffp-contract=off"
>  fi
>  AC_SUBST(MATH_FLAG)
>  


Re: Allow passing arrays in registers on AArch64

2014-02-18 Thread Michael Hudson-Doyle
Jakub Jelinek  writes:

> On Tue, Feb 11, 2014 at 02:51:08PM +, Marcus Shawcroft wrote:
>> On 6 February 2014 22:51, Michael Hudson-Doyle
>>  wrote:
>> 
>> > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> > index 16c51a8..958c667 100644
>> > --- a/gcc/config/aarch64/aarch64.c
>> > +++ b/gcc/config/aarch64/aarch64.c
>> > @@ -1187,14 +1187,10 @@ aarch64_pass_by_reference (cumulative_args_t pcum 
>> > ATTRIBUTE_UNUSED,
>> >size = (mode == BLKmode && type)
>> >  ? int_size_in_bytes (type) : (int) GET_MODE_SIZE (mode);
>> >
>> > -  if (type)
>> > +  /* Aggregates are passed by reference based on their size.  */
>> > +  if (type && AGGREGATE_TYPE_P (type))
>> >  {
>> > -  /* Arrays always passed by reference.  */
>> > -  if (TREE_CODE (type) == ARRAY_TYPE)
>> > -   return true;
>> > -  /* Other aggregates based on their size.  */
>> > -  if (AGGREGATE_TYPE_P (type))
>> > -   size = int_size_in_bytes (type);
>> > +  size = int_size_in_bytes (type);
>> >  }
>> >
>> >/* Variable sized arguments are always returned by reference.  */
>> 
>> This version of the patch looks fine.  Since this is a bug I think it
>> should be committed now in stage 4.This is OK if release manager
>> agrees.
>
> Ok.

So, um, can someone commit this please?

Cheers,
mwh


Re: Allow passing arrays in registers on AArch64

2014-02-06 Thread Michael Hudson-Doyle
Ramana Radhakrishnan  writes:

> On Tue, Feb 4, 2014 at 2:12 AM, Michael Hudson-Doyle
>  wrote:
>> Ping?  I'm attaching a marginally cleaner version of the test.  I've had
>> a look at integrating this into aapcs64.exp but got defeated in the
>> end.  If go-torture-execute took a list of sources as c-torture-execute
>> does, then I think adding something like this to aapcs64.exp would work:
>>
>> # Test passing arrays by value
>> set src $srcdir/$subdir/test_array.go
>> if {[runtest_file_p $runtests $src]} {
>> go-torture-execute [list $src $srcdir/$subdir/abitest.S 
>> $srcdir/$subdir/_test_array_c.c] \
>> $additional_flags
>> }
>>
>> but it doesn't.  I also think that some stuff needs to be set up before
>> go-torture-execute can be called that I don't really understand and I
>> also also don't know how to avoid executing this test if gccgo hasn't
>> been built.
>
> Putting in a shell script like that is a bad idea, 

To be clear: I was never proposing that this shell script be committed.
It was just an unambiguous way of showing how to run my test.

> the dejagnu infrastructure can take care of all the transport and
> target bits for this. This will fail if someone were to try testing
> this on qemu while the rest of the testsuite might work.
>
> However what happens if in aarch64.exp
>
> you do a
>
> load_lib go-dg.exp
>
> and essentially run the tests similar to testsuite/go.dg/dg.exp  ?

I can't see how to run a test case that consists of multiple source
files like that.  That doesn't mean it's not possible though...

> That will take care of any cross-testing issues I hope with this using
> dejagnu.
>
> If that fails I think we should just drop the test as the go testsuite
> will catch this.

Please.

>>
>> All that said, is there any chance of getting the original ABI fix
>> committed?  It would be nice to have it in 4.9.
>
>
> I cannot approve or disapprove this patch but looking at the fix and
> the ABI issue under question here, I agree that this should be fixed
> for 4.9 and documented in the release notes. Your latest patch should
> also take Yufeng's suggestion under consideration about merging the
> condition in the if block.

Oh right, I forgot that I hadn't sent a patch acting on that comment.
Here is one.

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 16c51a8..958c667 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -1187,14 +1187,10 @@ aarch64_pass_by_reference (cumulative_args_t pcum ATTRIBUTE_UNUSED,
   size = (mode == BLKmode && type)
 ? int_size_in_bytes (type) : (int) GET_MODE_SIZE (mode);
 
-  if (type)
+  /* Aggregates are passed by reference based on their size.  */
+  if (type && AGGREGATE_TYPE_P (type))
 {
-  /* Arrays always passed by reference.  */
-  if (TREE_CODE (type) == ARRAY_TYPE)
-	return true;
-  /* Other aggregates based on their size.  */
-  if (AGGREGATE_TYPE_P (type))
-	size = int_size_in_bytes (type);
+  size = int_size_in_bytes (type);
 }
 
   /* Variable sized arguments are always returned by reference.  */

Cheers,
mwh

>
> Cheers,
> Ramana
>
>>
>> Cheers,
>> mwh
>>
>> Michael Hudson-Doyle  writes:
>>
>>> Richard Earnshaw  writes:
>>>
>>>> On 17/01/14 23:56, Michael Hudson-Doyle wrote:
>>>>> Ian Lance Taylor  writes:
>>>>>
>>>>>> On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
>>>>>>  wrote:
>>>>>>>
>>>>>>> On 18 Jan 2014 07:50, "Yufeng Zhang"  wrote:
>>>>>>>>
>>>>>>>> Also can you please try to add some new test(s)?  It may not be that
>>>>>>>> straightforward to add non-C/C++ tests, but give it a try.
>>>>>>>
>>>>>>> Can you give some hints? Like at least where in the tree such a test 
>>>>>>> would
>>>>>>> go? I don't know this code at all.
>>>>>>
>>>>>> There is already a test in libgo, of course.
>>>>>>
>>>>>> I think it would be pretty hard to write a test that doesn't something
>>>>>> like what libgo does.  The problem is that GCC is entirely consistent
>>>>>> with and without your patch.  You could add a Go test that passes an
>>>>>> array in gcc/testsuite/go.go-torture/execute/ easily enough, but it
>>>>>> would be quite hard to add a test that doesn't pass wheth

Re: Allow passing arrays in registers on AArch64

2014-02-03 Thread Michael Hudson-Doyle
Ping?  I'm attaching a marginally cleaner version of the test.  I've had
a look at integrating this into aapcs64.exp but got defeated in the
end.  If go-torture-execute took a list of sources as c-torture-execute
does, then I think adding something like this to aapcs64.exp would work:

# Test passing arrays by value
set src $srcdir/$subdir/test_array.go
if {[runtest_file_p $runtests $src]} {
go-torture-execute [list $src $srcdir/$subdir/abitest.S 
$srcdir/$subdir/_test_array_c.c] \
$additional_flags
}

but it doesn't.  I also think that some stuff needs to be set up before
go-torture-execute can be called that I don't really understand and I
also also don't know how to avoid executing this test if gccgo hasn't
been built.

All that said, is there any chance of getting the original ABI fix
committed?  It would be nice to have it in 4.9.

Cheers,
mwh

Michael Hudson-Doyle  writes:

> Richard Earnshaw  writes:
>
>> On 17/01/14 23:56, Michael Hudson-Doyle wrote:
>>> Ian Lance Taylor  writes:
>>> 
>>>> On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
>>>>  wrote:
>>>>>
>>>>> On 18 Jan 2014 07:50, "Yufeng Zhang"  wrote:
>>>>>>
>>>>>> Also can you please try to add some new test(s)?  It may not be that
>>>>>> straightforward to add non-C/C++ tests, but give it a try.
>>>>>
>>>>> Can you give some hints? Like at least where in the tree such a test would
>>>>> go? I don't know this code at all.
>>>>
>>>> There is already a test in libgo, of course.
>>>>
>>>> I think it would be pretty hard to write a test that doesn't something
>>>> like what libgo does.  The problem is that GCC is entirely consistent
>>>> with and without your patch.  You could add a Go test that passes an
>>>> array in gcc/testsuite/go.go-torture/execute/ easily enough, but it
>>>> would be quite hard to add a test that doesn't pass whether or not
>>>> your patch is applied.
>>> 
>>> I think it would have to be a code generation test, i.e. that compiling
>>> something like
>>> 
>>> func second(e [2]int64) int64 {
>>> return e[1]
>>> }
>>> 
>>> does not access memory or something along those lines.  I'll have a look
>>> next week.
>>> 
>>> Cheers,
>>> mwh
>>> 
>>
>> Michael,
>>
>> Can you have a look at how the tests in gcc.target/aarch64/aapcs64 work;
>> it ought to be possible to write a test (though not in C) to catch this.
>
> Yeah, I had found those tests.  It would be much easier if I could write
> this in C :-)
>
>> The tests work by calling a wrapper function written in assembler --
>> that wrapper stores out all the registers used for parameter passing and
>> then calls back into the test's validator with the register dump
>> available.  Code can then check that values are passed in the places
>> expected.  This gives the compiler the separation between caller and
>> callee that's needed to test this feature.
>
> Right, that much makes sense.  I'm not sure I completely understand all
> the preprocessor trickery involved but the output of it seems clear
> enough.
>
>> My preferred languages for these tests would be (in approximate order)
>> c, c++, fortran, java, ada, go.  That order is based on which languages
>> are tested most by users.
>
> Well of course it cannot be done in C or C++.  A commenter on the PR
> said that while fortran does allow passing arrays by value, it's all
> handled in the frontend.  No idea about Java.  Ada has arrays by value
> but I don't know it even slightly.  So it's the last one on your list
> but here's a diff that adds hack at a test in Go:
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/abitest.S 
> b/gcc/testsuite/gcc.target/aarch64/aapcs64/abitest.S
> index 86ce7be..365cd91 100644
> --- a/gcc/testsuite/gcc.target/aarch64/aapcs64/abitest.S
> +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/abitest.S
> @@ -1,9 +1,12 @@
>   .global dumpregs
>   .global myfunc
> + .global main.myfunc
>   .type dumpregs,%function
>   .type myfunc,%function
> + .type main.myfunc,%function
>  dumpregs:
>  myfunc:
> +main.myfunc:
>movx16, sp
>movx17, sp
>subsp,  sp, 352 // 336 for registers and 16 for old sp and lr
> diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.go 
> b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.g

fix inconsistent install paths between gccgo and go tool

2014-01-21 Thread Michael Hudson-Doyle
Hi,

This patch for the 4.8 branch fixes an inconsistency between gccgo's
libgo and the go tool over where libraries installed with "go install
-compiler gccgo" end up.  Even if it's not strictly required, it makes
sense to me that as gccgo implements go 1.1 it should match the go tool
from that particular version of the go distribution (and also newer
ones!).

Cheers,
mwh


diff --git a/libgo/go/go/build/build.go b/libgo/go/go/build/build.go
index cc89afb..59ddcef 100644
--- a/libgo/go/go/build/build.go
+++ b/libgo/go/go/build/build.go
@@ -429,7 +429,7 @@ func (ctxt *Context) Import(path string, srcDir string, mode ImportMode) (*Packa
 	switch ctxt.Compiler {
 	case "gccgo":
 		dir, elem := pathpkg.Split(p.ImportPath)
-		pkga = "pkg/gccgo/" + dir + "lib" + elem + ".a"
+		pkga = "pkg/gccgo_" + ctxt.GOOS + "_" + ctxt.GOARCH + "/" + dir + "lib" + elem + ".a"
 	case "gc":
 		suffix := ""
 		if ctxt.InstallSuffix != "" {


Re: Allow passing arrays in registers on AArch64

2014-01-20 Thread Michael Hudson-Doyle
Richard Earnshaw  writes:

> On 17/01/14 23:56, Michael Hudson-Doyle wrote:
>> Ian Lance Taylor  writes:
>> 
>>> On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
>>>  wrote:
>>>>
>>>> On 18 Jan 2014 07:50, "Yufeng Zhang"  wrote:
>>>>>
>>>>> Also can you please try to add some new test(s)?  It may not be that
>>>>> straightforward to add non-C/C++ tests, but give it a try.
>>>>
>>>> Can you give some hints? Like at least where in the tree such a test would
>>>> go? I don't know this code at all.
>>>
>>> There is already a test in libgo, of course.
>>>
>>> I think it would be pretty hard to write a test that doesn't something
>>> like what libgo does.  The problem is that GCC is entirely consistent
>>> with and without your patch.  You could add a Go test that passes an
>>> array in gcc/testsuite/go.go-torture/execute/ easily enough, but it
>>> would be quite hard to add a test that doesn't pass whether or not
>>> your patch is applied.
>> 
>> I think it would have to be a code generation test, i.e. that compiling
>> something like
>> 
>> func second(e [2]int64) int64 {
>> return e[1]
>> }
>> 
>> does not access memory or something along those lines.  I'll have a look
>> next week.
>> 
>> Cheers,
>> mwh
>> 
>
> Michael,
>
> Can you have a look at how the tests in gcc.target/aarch64/aapcs64 work;
> it ought to be possible to write a test (though not in C) to catch this.

Yeah, I had found those tests.  It would be much easier if I could write
this in C :-)

> The tests work by calling a wrapper function written in assembler --
> that wrapper stores out all the registers used for parameter passing and
> then calls back into the test's validator with the register dump
> available.  Code can then check that values are passed in the places
> expected.  This gives the compiler the separation between caller and
> callee that's needed to test this feature.

Right, that much makes sense.  I'm not sure I completely understand all
the preprocessor trickery involved but the output of it seems clear
enough.

> My preferred languages for these tests would be (in approximate order)
> c, c++, fortran, java, ada, go.  That order is based on which languages
> are tested most by users.

Well of course it cannot be done in C or C++.  A commenter on the PR
said that while fortran does allow passing arrays by value, it's all
handled in the frontend.  No idea about Java.  Ada has arrays by value
but I don't know it even slightly.  So it's the last one on your list
but here's a diff that adds hack at a test in Go:

diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/abitest.S 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/abitest.S
index 86ce7be..365cd91 100644
--- a/gcc/testsuite/gcc.target/aarch64/aapcs64/abitest.S
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/abitest.S
@@ -1,9 +1,12 @@
.global dumpregs
.global myfunc
+   .global main.myfunc
.type dumpregs,%function
.type myfunc,%function
+   .type main.myfunc,%function
 dumpregs:
 myfunc:
+main.myfunc:
   mov  x16, sp
   mov  x17, sp
   sub  sp,  sp, 352 // 336 for registers and 16 for old sp and lr
diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.go 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.go
new file mode 100644
index 000..b5e90e4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.go
@@ -0,0 +1,9 @@
+package main
+
+func myfunc(e [2]int64)
+
+func main() {
+   myfunc([2]int64{40, 50})
+}
+
+
diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.sh 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.sh
new file mode 100755
index 000..0b3202d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.sh
@@ -0,0 +1,11 @@
+#!/bin/sh
+GCC=${GCC:-gcc}
+AARCH64HOST=${AARCH64HOST:-localhost}
+
+set -x
+
+${GCC}go -g -c test_array.go -o test_array.o
+${GCC} -g -c abitest.S -o abitest.o
+${GCC} -g -c test_array_c.c -o test_array_c.o
+${GCC}go -g abitest.o test_array.o test_array_c.o -o test_array
+scp ./test_array ${AARCH64HOST}: && ssh ${AARCH64HOST} ./test_array
diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array_c.c 
b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array_c.c
new file mode 100644
index 000..981d12d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array_c.c
@@ -0,0 +1,19 @@
+int which_kind_of_test = 0;
+
+#include "abitest-common.h"
+
+void
+testfunc (char *stack)
+{
+  {
+long __x = 40;
+if (memcmp (&__x, stack + X0, sizeof (long)) != 0)
+  abort ();
+  }
+  {
+long __x = 50;
+if (memcmp (&__x, stack + X1, sizeof (long)) != 0)
+  abort ();
+  }
+  return;
+}

Obviously it's not integrated into the test framework even slightly but
on the good side, it fails without my fix and passes with it... Are you
able to do the integration part or provide some hints for me?

Cheers,
mwh


Re: Allow passing arrays in registers on AArch64

2014-01-17 Thread Michael Hudson-Doyle
Ian Lance Taylor  writes:

> On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
>  wrote:
>>
>> On 18 Jan 2014 07:50, "Yufeng Zhang"  wrote:
>>>
>>> Also can you please try to add some new test(s)?  It may not be that
>>> straightforward to add non-C/C++ tests, but give it a try.
>>
>> Can you give some hints? Like at least where in the tree such a test would
>> go? I don't know this code at all.
>
> There is already a test in libgo, of course.
>
> I think it would be pretty hard to write a test that doesn't something
> like what libgo does.  The problem is that GCC is entirely consistent
> with and without your patch.  You could add a Go test that passes an
> array in gcc/testsuite/go.go-torture/execute/ easily enough, but it
> would be quite hard to add a test that doesn't pass whether or not
> your patch is applied.

I think it would have to be a code generation test, i.e. that compiling
something like

func second(e [2]int64) int64 {
return e[1]
}

does not access memory or something along those lines.  I'll have a look
next week.

Cheers,
mwh


Allow passing arrays in registers on AArch64

2014-01-17 Thread Michael Hudson-Doyle
Hi, as discussed in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59799
GCC currently gets a detail of the AArch64 ABI wrong: arrays are not
always passed by reference.  Fortunately the fix is rather easy...

I guess this is an ABI break but my understand there has been no release
of GCC which supports compiling a language that can pass arrays by value
on AArch64 yet.

Cheers,
mwh

  2014-01-17  Michael Hudson-Doyle  

PR target/59799

* config/aarch64/aarch64.c (aarch64_pass_by_reference):
  The rules for passing arrays in registers are the same as
  for structs, so remove the special case for them.

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index fa53c71..d63da95 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -987,10 +987,7 @@ aarch64_pass_by_reference (cumulative_args_t pcum ATTRIBUTE_UNUSED,
 
   if (type)
 {
-  /* Arrays always passed by reference.  */
-  if (TREE_CODE (type) == ARRAY_TYPE)
-	return true;
-  /* Other aggregates based on their size.  */
+  /* Aggregates based on their size.  */
   if (AGGREGATE_TYPE_P (type))
 	size = int_size_in_bytes (type);
 }


Re: [gofrontend-dev] libgo patch committed: Fix 32-bit memory allocation

2014-01-09 Thread Michael Hudson-Doyle

Ian Lance Taylor  writes:

> This patch to libgo fixes memory allocation on 32-bit systems when a lot
> of memory has been allocated.  The problem is described in this patch to
> the master repository: https://codereview.appspot.com/49460043 .

Here's a patch for the 4.8 branch if you are interested.  I haven't
tested it yet -- well, it's in progress but I'm not going to hang around
long enough for it to finish today.

Cheers,
mwh

diff --git a/libgo/runtime/malloc.goc b/libgo/runtime/malloc.goc
index 8ccaa6b..f0871dd 100644
--- a/libgo/runtime/malloc.goc
+++ b/libgo/runtime/malloc.goc
@@ -541,8 +541,7 @@ runtime_settype_flush(M *mp, bool sysalloc)
 
 		// (Manually inlined copy of runtime_MHeap_Lookup)
 		p = (uintptr)v>>PageShift;
-		if(sizeof(void*) == 8)
-			p -= (uintptr)runtime_mheap->arena_start >> PageShift;
+		p -= (uintptr)runtime_mheap->arena_start >> PageShift;
 		s = runtime_mheap->map[p];
 
 		if(s->sizeclass == 0) {
diff --git a/libgo/runtime/mgc0.c b/libgo/runtime/mgc0.c
index c3b3211..9f17bdc 100644
--- a/libgo/runtime/mgc0.c
+++ b/libgo/runtime/mgc0.c
@@ -239,8 +239,7 @@ markonly(void *obj)
 	// (Manually inlined copy of MHeap_LookupMaybe.)
 	k = (uintptr)obj>>PageShift;
 	x = k;
-	if(sizeof(void*) == 8)
-		x -= (uintptr)runtime_mheap->arena_start>>PageShift;
+	x -= (uintptr)runtime_mheap->arena_start>>PageShift;
 	s = runtime_mheap->map[x];
 	if(s == nil || k < s->start || k - s->start >= s->npages || s->state != MSpanInUse)
 		return false;
@@ -418,8 +417,7 @@ flushptrbuf(PtrTarget *ptrbuf, PtrTarget **ptrbufpos, Obj **_wp, Workbuf **_wbuf
 			// (Manually inlined copy of MHeap_LookupMaybe.)
 			k = (uintptr)obj>>PageShift;
 			x = k;
-			if(sizeof(void*) == 8)
-x -= (uintptr)arena_start>>PageShift;
+			x -= (uintptr)arena_start>>PageShift;
 			s = runtime_mheap->map[x];
 			if(s == nil || k < s->start || k - s->start >= s->npages || s->state != MSpanInUse)
 continue;
@@ -466,8 +464,7 @@ flushptrbuf(PtrTarget *ptrbuf, PtrTarget **ptrbufpos, Obj **_wp, Workbuf **_wbuf
 			// Ask span about size class.
 			// (Manually inlined copy of MHeap_Lookup.)
 			x = (uintptr)obj >> PageShift;
-			if(sizeof(void*) == 8)
-x -= (uintptr)arena_start>>PageShift;
+			x -= (uintptr)arena_start>>PageShift;
 			s = runtime_mheap->map[x];
 
 			PREFETCH(obj);
@@ -585,8 +582,7 @@ checkptr(void *obj, uintptr objti)
 	if(t == nil)
 		return;
 	x = (uintptr)obj >> PageShift;
-	if(sizeof(void*) == 8)
-		x -= (uintptr)(runtime_mheap->arena_start)>>PageShift;
+	x -= (uintptr)(runtime_mheap->arena_start)>>PageShift;
 	s = runtime_mheap->map[x];
 	objstart = (byte*)((uintptr)s->startstart + npage, s->npages - npage);
 		s->npages = npage;
 		p = t->start;
-		if(sizeof(void*) == 8)
-			p -= ((uintptr)h->arena_start>>PageShift);
+		p -= ((uintptr)h->arena_start>>PageShift);
 		if(p > 0)
 			h->map[p-1] = s;
 		h->map[p] = t;
@@ -169,8 +168,7 @@ HaveSpan:
 	s->elemsize = (sizeclass==0 ? s->npagesstart;
-	if(sizeof(void*) == 8)
-		p -= ((uintptr)h->arena_start>>PageShift);
+	p -= ((uintptr)h->arena_start>>PageShift);
 	for(n=0; nmap[p+n] = s;
 	return s;
@@ -241,8 +239,7 @@ MHeap_Grow(MHeap *h, uintptr npage)
 	mstats.mspan_sys = h->spanalloc.sys;
 	runtime_MSpan_Init(s, (uintptr)v>>PageShift, ask>>PageShift);
 	p = s->start;
-	if(sizeof(void*) == 8)
-		p -= ((uintptr)h->arena_start>>PageShift);
+	p -= ((uintptr)h->arena_start>>PageShift);
 	h->map[p] = s;
 	h->map[p + s->npages - 1] = s;
 	s->state = MSpanInUse;
@@ -259,8 +256,7 @@ runtime_MHeap_Lookup(MHeap *h, void *v)
 	uintptr p;
 	
 	p = (uintptr)v;
-	if(sizeof(void*) == 8)
-		p -= (uintptr)h->arena_start;
+	p -= (uintptr)h->arena_start;
 	return h->map[p >> PageShift];
 }
 
@@ -281,8 +277,7 @@ runtime_MHeap_LookupMaybe(MHeap *h, void *v)
 		return nil;
 	p = (uintptr)v>>PageShift;
 	q = p;
-	if(sizeof(void*) == 8)
-		q -= (uintptr)h->arena_start >> PageShift;
+	q -= (uintptr)h->arena_start >> PageShift;
 	s = h->map[q];
 	if(s == nil || p < s->start || p - s->start >= s->npages)
 		return nil;
@@ -332,8 +327,7 @@ MHeap_FreeLocked(MHeap *h, MSpan *s)
 
 	// Coalesce with earlier, later spans.
 	p = s->start;
-	if(sizeof(void*) == 8)
-		p -= (uintptr)h->arena_start >> PageShift;
+	p -= (uintptr)h->arena_start >> PageShift;
 	if(p > 0 && (t = h->map[p-1]) != nil && t->state != MSpanInUse) {
 		tp = (uintptr*)(t->start<

Re: [gofrontend-dev] Go patch committed: Implement method values in reflect package

2013-12-12 Thread Michael Hudson-Doyle
Ian Lance Taylor  writes:

> On Thu, Dec 12, 2013 at 12:21 AM, Michael Hudson-Doyle
>  wrote:
>> Ian Lance Taylor  writes:
>>
>>> This patch to the Go frontend and libgo implements method values in the
>>> reflect package.  Working with method values and reflect now works
>>> correctly, at least on x86.
>>
>> Can you give me a test case?  I can try it on a few other architectures
>> tomorrow.
>
> The test case is test/recover.go in the master Go sources.  But I know
> that that test won't work on other architectures, because currently
> reflect.MakeFunc is only implemented for 386 and amd64.

Ah, OK.

> It should be possible to implement reflect.MakeFunc for all
> architectures in a somewhat inefficient manner by using libffi's
> closure API.  I have not tried that.
>
> It is also of course possible to implement support for any specific
> processor as I did for 386 and amd64.  The difficulty depends on the
> difficulty of the ABI.  In general it's not too hard but it requires a
> clear understanding of ABI details and assembly language programming
> with full backtrace information.  It's about 600 lines of code for
> amd64.

OK, I'll add it to a list of things to look at "at some point"...

Cheers,
mwh


Re: [gofrontend-dev] Go patch committed: Implement method values in reflect package

2013-12-12 Thread Michael Hudson-Doyle
Ian Lance Taylor  writes:

> This patch to the Go frontend and libgo implements method values in the
> reflect package.  Working with method values and reflect now works
> correctly, at least on x86.

Can you give me a test case?  I can try it on a few other architectures
tomorrow.

Cheers,
mwh

> This changes the type signature for type methods in the reflect
> package to match the gc compiler.  That in turn required changing the
> reflect package to mark method values with a new flag, as previously
> they were detected by the type signature.  The MakeFunc support needed
> to create a function that takes a value and passes a pointer to the
> method, since all methods take pointers.  It also needed to create a
> function that holds a receiver value.  And the recover code needed to
> handle these new cases.  Bootstrapped and ran Go testsuite on
> x86_64-unknown-linux-gnu.  Committed to mainline and 4.8 branch.
>
> Ian


Re: [patch] introduce aarch64 as a Go architecture

2013-12-01 Thread Michael Hudson-Doyle
Ian Lance Taylor  writes:

> I've gotten a patch from Michael Hudson-Doyle to set GOARCH to arm64
> on an Aarch64 system (https://codereview.appspot.com/34830045/).  

Haha, go us.

> I've gotten a patch from Matthias Klose to set GOARCH to aarch64 on
> such a system
> (http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03765.html).
>
> I don't care one way or another myself, but we need to pick one.

I don't care too much myself.  AArch64 is more correct but arm64 is more
obvious.  Also plan9/inferno will use arm64 IIUC.

Cheers,
mwh


backport fix for go hash function names to 4.8

2013-11-27 Thread Michael Hudson-Doyle
Hi,

This patch brings the recent fix for the generated hash functions of
types that are aliases for structures containing unexported fields to
the 4.8 branch.

Cheers,
mwh

diff --git a/gcc/go/gofrontend/types.cc b/gcc/go/gofrontend/types.cc
index 59247d6..36383de 100644
--- a/gcc/go/gofrontend/types.cc
+++ b/gcc/go/gofrontend/types.cc
@@ -1834,7 +1834,9 @@ Type::write_specific_type_functions(Gogo* gogo, 
Named_type* name,
   bloc);
   gogo->start_block(bloc);
 
-  if (this->struct_type() != NULL)
+  if (name != NULL && name->real_type()->named_type() != NULL)
+this->write_named_hash(gogo, name, hash_fntype, equal_fntype);
+  else if (this->struct_type() != NULL)
 this->struct_type()->write_hash_function(gogo, name, hash_fntype,
 equal_fntype);
   else if (this->array_type() != NULL)
@@ -1852,7 +1854,9 @@ Type::write_specific_type_functions(Gogo* gogo, 
Named_type* name,
false, bloc);
   gogo->start_block(bloc);
 
-  if (this->struct_type() != NULL)
+  if (name != NULL && name->real_type()->named_type() != NULL)
+this->write_named_equal(gogo, name);
+  else if (this->struct_type() != NULL)
 this->struct_type()->write_equal_function(gogo, name);
   else if (this->array_type() != NULL)
 this->array_type()->write_equal_function(gogo, name);
@@ -1865,6 +1869,100 @@ Type::write_specific_type_functions(Gogo* gogo, 
Named_type* name,
   gogo->finish_function(bloc);
 }
 
+// Write a hash function that simply calls the hash function for a
+// named type.  This is used when one named type is defined as
+// another.  This ensures that this case works when the other named
+// type is defined in another package and relies on calling hash
+// functions defined only in that package.
+
+void
+Type::write_named_hash(Gogo* gogo, Named_type* name,
+  Function_type* hash_fntype, Function_type* equal_fntype)
+{
+  Location bloc = Linemap::predeclared_location();
+
+  Named_type* base_type = name->real_type()->named_type();
+  go_assert(base_type != NULL);
+
+  // The pointer to the type we are going to hash.  This is an
+  // unsafe.Pointer.
+  Named_object* key_arg = gogo->lookup("key", NULL);
+  go_assert(key_arg != NULL);
+
+  // The size of the type we are going to hash.
+  Named_object* keysz_arg = gogo->lookup("key_size", NULL);
+  go_assert(keysz_arg != NULL);
+
+  Named_object* hash_fn;
+  Named_object* equal_fn;
+  name->real_type()->type_functions(gogo, base_type, hash_fntype, equal_fntype,
+   &hash_fn, &equal_fn);
+
+  // Call the hash function for the base type.
+  Expression* key_ref = Expression::make_var_reference(key_arg, bloc);
+  Expression* keysz_ref = Expression::make_var_reference(keysz_arg, bloc);
+  Expression_list* args = new Expression_list();
+  args->push_back(key_ref);
+  args->push_back(keysz_ref);
+  Expression* func = Expression::make_func_reference(hash_fn, NULL, bloc);
+  Expression* call = Expression::make_call(func, args, false, bloc);
+
+  // Return the hash of the base type.
+  Expression_list* vals = new Expression_list();
+  vals->push_back(call);
+  Statement* s = Statement::make_return_statement(vals, bloc);
+  gogo->add_statement(s);
+}
+
+// Write an equality function that simply calls the equality function
+// for a named type.  This is used when one named type is defined as
+// another.  This ensures that this case works when the other named
+// type is defined in another package and relies on calling equality
+// functions defined only in that package.
+
+void
+Type::write_named_equal(Gogo* gogo, Named_type* name)
+{
+  Location bloc = Linemap::predeclared_location();
+
+  // The pointers to the types we are going to compare.  These have
+  // type unsafe.Pointer.
+  Named_object* key1_arg = gogo->lookup("key1", NULL);
+  Named_object* key2_arg = gogo->lookup("key2", NULL);
+  go_assert(key1_arg != NULL && key2_arg != NULL);
+
+  Named_type* base_type = name->real_type()->named_type();
+  go_assert(base_type != NULL);
+
+  // Build temporaries with the base type.
+  Type* pt = Type::make_pointer_type(base_type);
+
+  Expression* ref = Expression::make_var_reference(key1_arg, bloc);
+  ref = Expression::make_cast(pt, ref, bloc);
+  Temporary_statement* p1 = Statement::make_temporary(pt, ref, bloc);
+  gogo->add_statement(p1);
+
+  ref = Expression::make_var_reference(key2_arg, bloc);
+  ref = Expression::make_cast(pt, ref, bloc);
+  Temporary_statement* p2 = Statement::make_temporary(pt, ref, bloc);
+  gogo->add_statement(p2);
+
+  // Compare the values for equality.
+  Expression* t1 = Expression::make_temporary_reference(p1, bloc);
+  t1 = Expression::make_unary(OPERATOR_MULT, t1, bloc);
+
+  Expression* t2 = Expression::make_temporary_reference(p2, bloc);
+  t2 = Expression::make_unary(OPERATOR_MULT, t2, bloc);
+
+  Expression* cond = Expression::make_binary(OPERATOR_EQEQ, 

Backport reflect.Call fixes to 4.8 branch

2013-11-27 Thread Michael Hudson-Doyle
This patch brings the recent fix for calling a function or method that
takes or returns an empty struct via reflection to the 4.8 branch.

Cheers,
mwh

diff --git a/libgo/go/reflect/all_test.go b/libgo/go/reflect/all_test.go
index 526f09b..eecc459 100644
--- a/libgo/go/reflect/all_test.go
+++ b/libgo/go/reflect/all_test.go
@@ -1430,6 +1430,46 @@ func TestFunc(t *testing.T) {
}
 }
 
+type emptyStruct struct{}
+
+type nonEmptyStruct struct {
+   member int
+}
+
+func returnEmpty() emptyStruct {
+   return emptyStruct{}
+}
+
+func takesEmpty(e emptyStruct) {
+}
+
+func returnNonEmpty(i int) nonEmptyStruct {
+   return nonEmptyStruct{member: i}
+}
+
+func takesNonEmpty(n nonEmptyStruct) int {
+   return n.member
+}
+
+func TestCallWithStruct(t *testing.T) {
+   r := ValueOf(returnEmpty).Call([]Value{})
+   if len(r) != 1 || r[0].Type() != TypeOf(emptyStruct{}) {
+   t.Errorf("returning empty struct returned %s instead", r)
+   }
+   r = ValueOf(takesEmpty).Call([]Value{ValueOf(emptyStruct{})})
+   if len(r) != 0 {
+   t.Errorf("takesEmpty returned values: %s", r)
+   }
+   r = ValueOf(returnNonEmpty).Call([]Value{ValueOf(42)})
+   if len(r) != 1 || r[0].Type() != TypeOf(nonEmptyStruct{}) || 
r[0].Field(0).Int() != 42 {
+   t.Errorf("returnNonEmpty returned %s", r)
+   }
+   r = ValueOf(takesNonEmpty).Call([]Value{ValueOf(nonEmptyStruct{member: 
42})})
+   if len(r) != 1 || r[0].Type() != TypeOf(1) || r[0].Int() != 42 {
+   t.Errorf("takesNonEmpty returned %s", r)
+   }
+}
+
 func TestMakeFunc(t *testing.T) {
switch runtime.GOARCH {
case "amd64", "386":
diff --git a/libgo/runtime/go-reflect-call.c b/libgo/runtime/go-reflect-call.c
index 5cf3707..12bd0d7 100644
--- a/libgo/runtime/go-reflect-call.c
+++ b/libgo/runtime/go-reflect-call.c
@@ -98,9 +98,12 @@ go_struct_to_ffi (const struct __go_struct_type *descriptor)
   const struct __go_struct_field *fields;
   int i;
 
+  field_count = descriptor->__fields.__count;
+  if (field_count == 0) {
+return &ffi_type_void;
+  }
   ret = (ffi_type *) __go_alloc (sizeof (ffi_type));
   ret->type = FFI_TYPE_STRUCT;
-  field_count = descriptor->__fields.__count;
   fields = (const struct __go_struct_field *) descriptor->__fields.__values;
   ret->elements = (ffi_type **) __go_alloc ((field_count + 1)
* sizeof (ffi_type *));


Backport syslist.go fixes to 4.8

2013-11-27 Thread Michael Hudson-Doyle
Hi,

Recently, build.goosList and build.goarchList got fixed in mainline to
be sensible, hard-coded, lists rather than odd autogenerated lists.
This patch updates the 4.8 branch to match.

Cheers,
mwh

diff --git a/libgo/Makefile.am b/libgo/Makefile.am
index 957f23c..199b444 100644
--- a/libgo/Makefile.am
+++ b/libgo/Makefile.am
@@ -1255,7 +1255,7 @@ go_go_build_files = \
go/go/build/build.go \
go/go/build/doc.go \
go/go/build/read.go \
-   syslist.go
+   go/go/build/syslist.go
 go_go_doc_files = \
go/go/doc/comment.go \
go/go/doc/doc.go \
@@ -2713,15 +2713,6 @@ go/build/check: $(CHECK_DEPS)
@$(CHECK)
 .PHONY: go/build/check
 
-syslist.go: s-syslist; @true
-s-syslist: Makefile
-   echo '// Generated automatically by make.' >syslist.go.tmp
-   echo 'package build' >>syslist.go.tmp
-   echo 'const goosList = "$(GOOS)"' >>syslist.go.tmp
-   echo 'const goarchList = "$(GOARCH)"' >>syslist.go.tmp
-   $(SHELL) $(srcdir)/../move-if-change syslist.go.tmp syslist.go
-   $(STAMP) $@
-
 @go_include@ go/doc.lo.dep
 go/doc.lo.dep: $(go_go_doc_files)
$(BUILDDEPS)
diff --git a/libgo/Makefile.in b/libgo/Makefile.in
index 706a72e..61cca73 100644
--- a/libgo/Makefile.in
+++ b/libgo/Makefile.in
@@ -1447,7 +1447,7 @@ go_go_build_files = \
go/go/build/build.go \
go/go/build/doc.go \
go/go/build/read.go \
-   syslist.go
+   go/go/build/syslist.go
 
 go_go_doc_files = \
go/go/doc/comment.go \
@@ -5071,15 +5071,6 @@ go/build/check: $(CHECK_DEPS)
@$(CHECK)
 .PHONY: go/build/check
 
-syslist.go: s-syslist; @true
-s-syslist: Makefile
-   echo '// Generated automatically by make.' >syslist.go.tmp
-   echo 'package build' >>syslist.go.tmp
-   echo 'const goosList = "$(GOOS)"' >>syslist.go.tmp
-   echo 'const goarchList = "$(GOARCH)"' >>syslist.go.tmp
-   $(SHELL) $(srcdir)/../move-if-change syslist.go.tmp syslist.go
-   $(STAMP) $@
-
 @go_include@ go/doc.lo.dep
 go/doc.lo.dep: $(go_go_doc_files)
$(BUILDDEPS)
diff --git a/libgo/go/go/build/syslist.go b/libgo/go/go/build/syslist.go
new file mode 100644
index 000..a322c88
--- /dev/null
+++ b/libgo/go/go/build/syslist.go
@@ -0,0 +1,8 @@
+// Copyright 2011 The Go Authors.  All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package build
+
+const goosList = "darwin dragonfly freebsd linux netbsd openbsd plan9 windows 
solaris "
+const goarchList = "386 amd64 arm alpha m68k mipso32 mipsn32 mipsn64 mipso64 
ppc ppc64 sparc sparc64 "



Re: [gofrontend-dev] Moved gccgo branch to mainline

2013-11-06 Thread Michael Hudson-Doyle
Ian Lance Taylor  writes:

> I removed the gccgo branch based on the GCC 4.8 branch, and then
> recreated it as a copy of mainline.  Future changes to the gccgo
> branch will be merged from mainline.

This is probably a silly question, but presuamably bug fixes to the 4.8
gccgo will continue to land on gcc's 4.8 branch?

Cheers,
mwh


[RESEND] Enable building of libatomic on AArch64

2013-10-17 Thread Michael Hudson-Doyle
Resending as the previous attempt went missing...

  2013-10-04  Michael Hudson-Doyle  

* libatomic/configure.tgt (aarch64*): Remove code preventing
  build.

* gcc/testsuite/lib/target-supports.exp
  (check_effective_target_sync_long_long): AArch64 supports
  atomic operations on "long long".
  (check_effective_target_sync_long_long_runtime): AArch64 can
  execute atomic operations on "long long".

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 7eb4dfe..5557c06 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -4508,6 +4508,7 @@ proc check_effective_target_sync_int_128_runtime { } {
 proc check_effective_target_sync_long_long { } {
 if { [istarget x86_64-*-*]
 	 || [istarget i?86-*-*])
+	 || [istarget aarch64*-*-*]
 	 || [istarget arm*-*-*]
 	 || [istarget alpha*-*-*]
 	 || ([istarget sparc*-*-*] && [check_effective_target_lp64]) } {
@@ -4537,6 +4538,8 @@ proc check_effective_target_sync_long_long_runtime { } {
 		}
 	} ""
 	}]
+} elseif { [istarget aarch64*-*-*] } {
+	return 1
 } elseif { [istarget arm*-*-linux-*] } {
 	return [check_runtime sync_longlong_runtime {
 	#include 
diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
index b9e5d6c..7eaab38 100644
--- a/libatomic/configure.tgt
+++ b/libatomic/configure.tgt
@@ -95,11 +95,6 @@ fi
 
 # Other system configury
 case "${target}" in
-  aarch64*)
-	# This is currently not supported in AArch64.
-	UNSUPPORTED=1
-	;;
-
   arm*-*-linux*)
 	# OS support for atomic primitives.
 	config_path="${config_path} linux/arm posix"


Re: Enable building of libatomic on AArch64

2013-10-17 Thread Michael Hudson-Doyle
Ping?

Michael Hudson-Doyle  writes:

> Marcus Shawcroft  writes:
>
>> On 3 October 2013 23:43, Michael Hudson-Doyle  
>> wrote:
>>> Hi,
>>>
>>> As libatomic builds for and the tests pass on AArch64 (built on x86_64
>>> but tested on a foundation model, logs and summary:
>>>
>>> http://people.linaro.org/~mwhudson/libatomic.sum.txt
>>> http://people.linaro.org/~mwhudson/runtest-log-v-2.txt
>>>
>>> ) this patch enables the build.
>>>
>>> Cheers,
>>> mwh
>>> (first time posting to this list, let me know if I'm doing it wrong)
>>>
>>> 2013-10-04  Michael Hudson-Doyle  
>>>
>>>   * configure.tgt: Add AArch64 support.
>>>
>>
>> Hi,
>> The patch looks fine to me.
>
> Thanks for looking!
>
>> The ChangeLog entry should reflect the code that was removed rather
>> than the functionality added.  Perhaps:
>>
>>   * configure.tgt (aarch64*): Remove.
>
> There are few too many negatives going on to make a pithy explanation
> easy...
>
>> Did you investigate whether or not the 10 UNSUPPORTED results in the
>> testsuite are sane?
>
> I did not, but have now.
>
>> I think that 5 look legitimate since they require 128 bit sync ops.
>> The other 5 look superficially like they should be supported on
>> aarch64.  We may just be missing aarch64 target supports wiring in
>> check_effective_target_sync_long_long_runtime?
>
> Yes, that was it, with appropriate changes the -4 tests all pass.
>
> However, just out of a sense of curiosity, I added wiring to claim
> aarch64* supports 128 bit sync ops and all the -5 tests pass too.  Is
> that just luck or because the reservation granule on the foundation
> model is big enough or something else?
>
> In any case, I'll attach a patch that just claims support for long long
> sync ops for now...
>
> Cheers,
> mwh
>
>> /Marcus
>
> 2013-10-04  Michael Hudson-Doyle  
>
>   * libatomic/configure.tgt (aarch64*): Remove code preventing
> build.
>
>   * gcc/testsuite/lib/target-supports.exp
> (check_effective_target_sync_long_long): AArch64 supports
> atomic operations on "long long".
> (check_effective_target_sync_long_long_runtime): AArch64 can
> execute atomic operations on "long long".
>
> diff --git a/gcc/testsuite/lib/target-supports.exp 
> b/gcc/testsuite/lib/target-supports.exp
> index 7eb4dfe..5557c06 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -4508,6 +4508,7 @@ proc check_effective_target_sync_int_128_runtime { } {
>  proc check_effective_target_sync_long_long { } {
>  if { [istarget x86_64-*-*]
>|| [istarget i?86-*-*])
> +  || [istarget aarch64*-*-*]
>|| [istarget arm*-*-*]
>|| [istarget alpha*-*-*]
>|| ([istarget sparc*-*-*] && [check_effective_target_lp64]) } {
> @@ -4537,6 +4538,8 @@ proc check_effective_target_sync_long_long_runtime { } {
>   }
>   } ""
>   }]
> +} elseif { [istarget aarch64*-*-*] } {
> + return 1
>  } elseif { [istarget arm*-*-linux-*] } {
>   return [check_runtime sync_longlong_runtime {
>   #include 
> diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
> index b9e5d6c..7eaab38 100644
> --- a/libatomic/configure.tgt
> +++ b/libatomic/configure.tgt
> @@ -95,11 +95,6 @@ fi
>  
>  # Other system configury
>  case "${target}" in
> -  aarch64*)
> - # This is currently not supported in AArch64.
> - UNSUPPORTED=1
> - ;;
> -
>arm*-*-linux*)
>   # OS support for atomic primitives.
>   config_path="${config_path} linux/arm posix"


Enable building of libatomic on AArch64

2013-10-03 Thread Michael Hudson-Doyle
Hi,

As libatomic builds for and the tests pass on AArch64 (built on x86_64
but tested on a foundation model, logs and summary:

http://people.linaro.org/~mwhudson/libatomic.sum.txt
http://people.linaro.org/~mwhudson/runtest-log-v-2.txt

) this patch enables the build.

Cheers,
mwh
(first time posting to this list, let me know if I'm doing it wrong)

2013-10-04  Michael Hudson-Doyle  

  * configure.tgt: Add AArch64 support.

>From c01bc2acde08f21f23465dfd0d4d0e88adc6e214 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle 
Date: Thu, 12 Sep 2013 16:18:00 +1200
Subject: [PATCH] libatomic is now supported on AArch64

---
 libatomic/configure.tgt | 5 -
 1 file changed, 5 deletions(-)

diff --git a/libatomic/configure.tgt b/libatomic/configure.tgt
index b9e5d6c..7eaab38 100644
--- a/libatomic/configure.tgt
+++ b/libatomic/configure.tgt
@@ -95,11 +95,6 @@ fi
 
 # Other system configury
 case "${target}" in
-  aarch64*)
-	# This is currently not supported in AArch64.
-	UNSUPPORTED=1
-	;;
-
   arm*-*-linux*)
 	# OS support for atomic primitives.
 	config_path="${config_path} linux/arm posix"
-- 
1.8.1.2