Why do we need fno-omit-frame-pointer on aarch64 ?
Ramana
From: James Greenhalgh
Sent: Friday, 17 November, 22:02
Subject: Re: [COMMITTED][AArch64] Fix frame tests
To: Wilco Dijkstra
Cc: GCC Patches, nd, Richard Earnshaw, Marcus Shawcroft, Ramana Radhakrishnan
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
