From patchwork Tue Feb 4 02:12:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael-Doyle Hudson X-Patchwork-Id: 24078 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ob0-f200.google.com (mail-ob0-f200.google.com [209.85.214.200]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id F15D720445 for ; Tue, 4 Feb 2014 02:13:14 +0000 (UTC) Received: by mail-ob0-f200.google.com with SMTP id wo20sf31308290obc.7 for ; Mon, 03 Feb 2014 18:13:13 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mailing-list:precedence:list-id :list-unsubscribe:list-archive:list-post:list-help:sender :delivered-to:from:to:cc:subject:in-reply-to:references:user-agent :date:message-id:mime-version:x-original-sender :x-original-authentication-results:content-type; bh=+2wwWbQGpov3+k28l9zyOaMhxJIOYcFu3m5ooG8pg8U=; b=jnu+FB9fAHhii/5Pl51XRkPiPgtYw1AFoTUbE3Kg4OqizjktEzHj/D0r+T4WgpUfFM zTQTwMzeYRC5mTv3iNpzXHHwvAZd97Q/BJAZnEmUO/jF54pwyHmyPFLX0FZfWFvU3pdL FLwA/nS5NRc+innHPfbYm7Zv+XKKlE7k35e7VfRTt+BWDBV24ym+t/fIYRY/VsbPVgx5 gCqfFXJYFSlOsfmeMU4VTkULm5MZs2+mzCmwyd9XWOOg3LVdQCIPA0Ie1q+wzudj6B3y JDHInN3aYfiNGEadEmXP2EhaFNU69JnOyS59x/aGonPgcSC0onqrCy97hrt0SEl5/jtG LTkQ== X-Gm-Message-State: ALoCoQmpuZH0byhSXlUZKo0FBda4X/4ITSfyaHeEIELzc9MVSX565QYuOfgFkO5ADTEYwNmAyYoj X-Received: by 10.50.92.3 with SMTP id ci3mr6908625igb.5.1391479993779; Mon, 03 Feb 2014 18:13:13 -0800 (PST) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.89.145 with SMTP id v17ls2158892qgd.82.gmail; Mon, 03 Feb 2014 18:13:13 -0800 (PST) X-Received: by 10.220.97.145 with SMTP id l17mr225125vcn.35.1391479993647; Mon, 03 Feb 2014 18:13:13 -0800 (PST) Received: from mail-vb0-x230.google.com (mail-vb0-x230.google.com [2607:f8b0:400c:c02::230]) by mx.google.com with ESMTPS id xe7si7484905vec.40.2014.02.03.18.13.13 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 03 Feb 2014 18:13:13 -0800 (PST) Received-SPF: neutral (google.com: 2607:f8b0:400c:c02::230 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=2607:f8b0:400c:c02::230; Received: by mail-vb0-f48.google.com with SMTP id q16so5215631vbe.21 for ; Mon, 03 Feb 2014 18:13:13 -0800 (PST) X-Received: by 10.220.161.132 with SMTP id r4mr3392955vcx.29.1391479993440; Mon, 03 Feb 2014 18:13:13 -0800 (PST) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.220.174.196 with SMTP id u4csp199948vcz; Mon, 3 Feb 2014 18:13:11 -0800 (PST) X-Received: by 10.66.139.169 with SMTP id qz9mr41488041pab.16.1391479991097; Mon, 03 Feb 2014 18:13:11 -0800 (PST) Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id n8si22591083pax.305.2014.02.03.18.13.10 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 03 Feb 2014 18:13:11 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-360995-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Received: (qmail 11298 invoked by alias); 4 Feb 2014 02:12:58 -0000 Mailing-List: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 11285 invoked by uid 89); 4 Feb 2014 02:12:56 -0000 X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: youngberry.canonical.com Received: from youngberry.canonical.com (HELO youngberry.canonical.com) (91.189.89.112) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Feb 2014 02:12:53 +0000 Received: from [120.136.5.22] (helo=narsil) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1WAVVK-0000uh-7o; Tue, 04 Feb 2014 02:12:51 +0000 Received: by narsil (Postfix, from userid 1000) id AAECB52088C; Tue, 4 Feb 2014 15:12:45 +1300 (NZDT) From: Michael Hudson-Doyle To: Richard Earnshaw Cc: Ian Lance Taylor , Yufeng Zhang , Marcus Shawcroft , gcc-patches Subject: Re: Allow passing arrays in registers on AArch64 In-Reply-To: <874n4ynxkc.fsf@canonical.com> References: <87ha93nhl1.fsf@canonical.com> <52D97B6C.8010107@arm.com> <87eh46nowk.fsf@canonical.com> <52DCF158.8060708@arm.com> <874n4ynxkc.fsf@canonical.com> User-Agent: Notmuch/0.17~rc1+4~g7f07bfd (http://notmuchmail.org) Emacs/24.3.50.2 (x86_64-pc-linux-gnu) Date: Tue, 04 Feb 2014 15:12:45 +1300 Message-ID: <87ppn3fwwy.fsf@canonical.com> MIME-Version: 1.0 X-Original-Sender: michael.hudson@linaro.org X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 2607:f8b0:400c:c02::230 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@gcc.gnu.org X-Google-Group-Id: 836684582541 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: > 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; > +} > > 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 --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..c795e88 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.go @@ -0,0 +1,10 @@ +package main + +//extern myfunc +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..1c41192 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array.sh @@ -0,0 +1,8 @@ +#!/bin/sh +GCCGO=${GCCGO:-gccgo} +AARCH64HOST=${AARCH64HOST:-localhost} + +set -x + +${GCCGO} -g test_array.go abitest.S 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..41066b2 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/test_array_c.c @@ -0,0 +1,20 @@ +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; +} +