diff mbox

Allow passing arrays in registers on AArch64

Message ID 874n4ynxkc.fsf@canonical.com
State New
Headers show

Commit Message

Michael-Doyle Hudson Jan. 20, 2014, 9:38 p.m. UTC 
Richard Earnshaw <rearnsha@arm.com> writes:

> On 17/01/14 23:56, Michael Hudson-Doyle wrote:
>> Ian Lance Taylor <iant@golang.org> writes:
>> 
>>> On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
>>> <michael.hudson@canonical.com> wrote:
>>>>
>>>> On 18 Jan 2014 07:50, "Yufeng Zhang" <Yufeng.Zhang@arm.com> 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:


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
diff mbox

Patch

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 0000000..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 0000000..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 0000000..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;
+}