This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [COMMITTED][AArch64] Fix frame tests
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>, <richard dot earnshaw at arm dot com>, <marcus dot shawcroft at arm dot com>, <ramana dot radhakrishnan at arm dot com>
- Date: Fri, 17 Nov 2017 22:02:03 +0000
- Subject: Re: [COMMITTED][AArch64] Fix frame tests
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=pass (sender IP is 217.140.96.140) smtp.mailfrom=arm.com; gcc.gnu.org; dkim=none (message not signed) header.d=none;gcc.gnu.org; dmarc=bestguesspass action=none header.from=arm.com;
- Nodisclaimer: True
- References: <DB6PR0801MB2053190F6A7C547DB3DBC2E8832E0@DB6PR0801MB2053.eurprd08.prod.outlook.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On Thu, Nov 16, 2017 at 11:34:46AM +0000, Wilco Dijkstra wrote:
> Improve the AArch64 frame tests - add -f(no-)omit-frame-pointer,
> update checks and add missing tests. As a result all tests now
> pass.
>
> Committed as obvious.
Some of these are far from obvious... Even if they were obvious to you I'd
have appreciated a short description of each change.
For example...
> diff --git a/gcc/testsuite/gcc.target/aarch64/lr_free_2.c b/gcc/testsuite/gcc.target/aarch64/lr_free_2.c
> index e2b9490fab1a27755d239ad6802325a619f73db3..5d9500f4fb144bdae5d0199f0b0a218deb504176 100644
> --- a/gcc/testsuite/gcc.target/aarch64/lr_free_2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/lr_free_2.c
> @@ -1,5 +1,5 @@
> /* { dg-do run } */
> -/* { dg-options "-fno-inline -O2 -ffixed-x2 -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 -ffixed-x7 -ffixed-x8 -ffixed-x9 -ffixed-x10 -ffixed-x11 -ffixed-x12 -ffixed-x13 -ffixed-x14 -ffixed-x15 -ffixed-x16 -ffixed-x17 -ffixed-x18 -ffixed-x19 -ffixed-x20 -ffixed-x21 -ffixed-x22 -ffixed-x23 -ffixed-x24 -ffixed-x25 -ffixed-x26 -ffixed-x27 -ffixed-x28 --save-temps -mgeneral-regs-only -fno-ipa-cp -fdump-rtl-ira" } */
> +/* { dg-options "-fno-omit-frame-pointer -fno-inline -O2 -ffixed-x2 -ffixed-x3 -ffixed-x4 -ffixed-x5 -ffixed-x6 -ffixed-x7 -ffixed-x8 -ffixed-x9 -ffixed-x10 -ffixed-x11 -ffixed-x12 -ffixed-x13 -ffixed-x14 -ffixed-x15 -ffixed-x16 -ffixed-x17 -ffixed-x18 -ffixed-x19 -ffixed-x20 -ffixed-x21 -ffixed-x22 -ffixed-x23 -ffixed-x24 -ffixed-x25 -ffixed-x26 -ffixed-x27 -ffixed-x28 --save-temps -mgeneral-regs-only -fno-ipa-cp -fdump-rtl-ira" } */
>
> extern void abort ();
This test failing and now requiring -fno-omit-frame-pointer implies we have
changed the default of when we omit leaf frame pointers. We may have done
that deliberately, but the testcase change isn't obvious on its own.
> diff --git a/gcc/testsuite/gcc.target/aarch64/spill_1.c b/gcc/testsuite/gcc.target/aarch64/spill_1.c
> index 847425895d456e4433b0d15556d60a66a8f8f70c..c9528cb21daaefcdd5f1218ee13edf40ee44bd99 100644
> --- a/gcc/testsuite/gcc.target/aarch64/spill_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/spill_1.c
> @@ -14,5 +14,3 @@ foo (void)
> }
>
> /* { dg-final { scan-assembler-times {\tmovi\tv[0-9]+\.4s,} 2 } } */
> -/* { dg-final { scan-assembler-not {\tldr\t} } } */
> -/* { dg-final { scan-assembler-not {\tstr\t} } } */
This change doesn't seem to be needed at all?
And so on...
Even if there were broad classes of changes you made to the tests a
justification would be helpful.
The "obvious" rule needs to only apply when the patch would be obvious to
most of the community. I haven't reverted the patch (this time), but I would
appreciate a case-by-case explanation of the patch, if nothing else so we
can understandcyour motives here a few years in the future when we next change
these tests.
Thanks,
James