Re: Allow passing arrays in registers on AArch64
Jakub Jelinek ja...@redhat.com writes:
On Tue, Feb 11, 2014 at 02:51:08PM +, Marcus Shawcroft wrote:
On 6 February 2014 22:51, Michael Hudson-Doyle
michael.hud...@canonical.com 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
On 6 February 2014 22:51, Michael Hudson-Doyle
michael.hud...@canonical.com 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.
Cheers
/Marcus
Re: Allow passing arrays in registers on AArch64
On Tue, Feb 11, 2014 at 02:51:08PM +, Marcus Shawcroft wrote:
On 6 February 2014 22:51, Michael Hudson-Doyle
michael.hud...@canonical.com 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.
Jakub
Re: Allow passing arrays in registers on AArch64
On Tue, Feb 4, 2014 at 2:12 AM, Michael Hudson-Doyle
michael.hud...@linaro.org 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, 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 ?
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.
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.
Cheers,
Ramana
Cheers,
mwh
Michael Hudson-Doyle michael.hud...@linaro.org writes:
Richard Earnshaw rearn...@arm.com writes:
On 17/01/14 23:56, Michael Hudson-Doyle wrote:
Ian Lance Taylor i...@golang.org writes:
On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
michael.hud...@canonical.com wrote:
On 18 Jan 2014 07:50, Yufeng Zhang yufeng.zh...@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:
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 //
Re: Allow passing arrays in registers on AArch64
Ramana Radhakrishnan ramana@googlemail.com writes:
On Tue, Feb 4, 2014 at 2:12 AM, Michael Hudson-Doyle
michael.hud...@linaro.org 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 michael.hud...@linaro.org writes:
Richard Earnshaw rearn...@arm.com writes:
On 17/01/14 23:56, Michael Hudson-Doyle wrote:
Ian Lance Taylor i...@golang.org writes:
On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
michael.hud...@canonical.com wrote:
On 18 Jan 2014 07:50, Yufeng Zhang yufeng.zh...@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
Re: Allow passing arrays in registers on AArch64
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 michael.hud...@linaro.org writes:
Richard Earnshaw rearn...@arm.com writes:
On 17/01/14 23:56, Michael Hudson-Doyle wrote:
Ian Lance Taylor i...@golang.org writes:
On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
michael.hud...@canonical.com wrote:
On 18 Jan 2014 07:50, Yufeng Zhang yufeng.zh...@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:
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.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
Re: Allow passing arrays in registers on AArch64
Am 17.01.2014 19:50, schrieb Yufeng Zhang: Hi Michael, Thanks for the fix. The patch looks OK to me in general, although I have some minor comments below. On 01/17/14 08:22, Michael Hudson-Doyle wrote: Hi, as discussed inhttp://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... Can you please indicate what kind of testing you have run, e.g. regtest on aarch64-none-abi? Test with the trunk (all languages except Ada) with and without this patch: Running target unix FAIL: math FAIL: net -FAIL: reflect FAIL: runtime === libgo Summary === -# of expected passes 118 -# of unexpected failures 4 +# of expected passes 119 +# of unexpected failures 3 /home/doko/gcc/gcc-4.9-4.9-20140122/build/./gcc/gccgo version 4.9.0 20140122 (experimental) [trunk revision 206940] (Ubuntu 4.9-20140122-1ubuntu1) no other changes for the tests. I did test with the Linaro 4.8 branch, including FSF changes up to r206935, with and without this patch: -FAIL: io -FAIL: os -FAIL: reflect -FAIL: sync -FAIL: time -FAIL: archive/zip -FAIL: database/sql -FAIL: encoding/base64 -FAIL: go/doc -FAIL: go/printer -FAIL: log/syslog -FAIL: net/http/cgi -FAIL: net/http/httputil -FAIL: net/rpc -FAIL: net/rpc/jsonrpc -FAIL: old/template -FAIL: os/exec -FAIL: os/signal -FAIL: sync/atomic -FAIL: text/template -# of expected passes 99 -# of unexpected failures 23 +# of expected passes 119 +# of unexpected failures 3 there are some XPASS's for quality tests, and FAIL's for the pr36728 quality tests, however in the past these were always a bit noisy, succeeding or failing for various builds. I do see additional fails for: +FAIL: gcc.dg/simulate-thread/atomic-other-int.c -O0 -g thread simulation test +FAIL: gcc.dg/simulate-thread/atomic-other-int.c -O2 -g thread simulation test +FAIL: gcc.dg/simulate-thread/atomic-other-int.c -O3 -g thread simulation test +FAIL: gcc.dg/simulate-thread/atomic-other-short.c -O0 -g thread simulation test +FAIL: gcc.dg/simulate-thread/atomic-other-short.c -O2 -g thread simulation test +FAIL: gcc.dg/simulate-thread/atomic-other-short.c -O3 -g thread simulation test
Re: Allow passing arrays in registers on AArch64
On 17/01/14 23:56, Michael Hudson-Doyle wrote:
Ian Lance Taylor i...@golang.org writes:
On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
michael.hud...@canonical.com wrote:
On 18 Jan 2014 07:50, Yufeng Zhang yufeng.zh...@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.
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.
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.
R.
Re: Allow passing arrays in registers on AArch64
Richard Earnshaw rearn...@arm.com writes:
On 17/01/14 23:56, Michael Hudson-Doyle wrote:
Ian Lance Taylor i...@golang.org writes:
On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
michael.hud...@canonical.com wrote:
On 18 Jan 2014 07:50, Yufeng Zhang yufeng.zh...@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:
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
Michael == Michael Hudson-Doyle michael.hud...@linaro.org writes: 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. Michael Well of course it cannot be done in C or C++. A commenter on the PR Michael said that while fortran does allow passing arrays by value, it's all Michael handled in the frontend. No idea about Java. You cannot pass arrays by value in Java. In Java an array is a subclass of Object; in gcc internals terms it is just a pointer. Tom
Re: Allow passing arrays in registers on AArch64
Hi Michael,
Thanks for the fix. The patch looks OK to me in general, although I
have some minor comments below.
On 01/17/14 08:22, Michael Hudson-Doyle wrote:
Hi, as discussed inhttp://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...
Can you please indicate what kind of testing you have run, e.g. regtest
on aarch64-none-abi?
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.
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-Doylemichael.hud...@linaro.org
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.
aarch64-abi-fix.diff
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);
}
You can actually merge the two iffs to have something like:
/* Aggregates are based on their size. */
if (type AGGREGATE_TYPE_P (type))
size = int_size_in_bytes (type);
Thanks,
Yufeng
Re: Allow passing arrays in registers on AArch64
On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle michael.hud...@canonical.com wrote: On 18 Jan 2014 07:50, Yufeng Zhang yufeng.zh...@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. Ian
Re: Allow passing arrays in registers on AArch64
Ian Lance Taylor i...@golang.org writes:
On Fri, Jan 17, 2014 at 11:32 AM, Michael Hudson-Doyle
michael.hud...@canonical.com wrote:
On 18 Jan 2014 07:50, Yufeng Zhang yufeng.zh...@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
