diff mbox series

testsuite: xfail scev-[35].c on ia32

Message ID orjzqsi68k.fsf@lxoliva.fsfla.org
State Accepted
Headers
Series testsuite: xfail scev-[35].c on ia32 |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Alexandre Oliva Nov. 8, 2023, 4:01 p.m. UTC 
  These gimplefe tests never got the desired optimization on ia32, but
they only started visibly failing when the representation of MEMs in
dumps changed from printing 'symbol: a' to '&a'.

The transformation is not considered profitable on ia32, that's why it
doesn't take place.  Maybe that's a bug in itself, but it's not a
regression, and not something to be noisy about.

Regstrapped on x86_64-linux-gnu, also tested with gcc-13 on i686- and
x86_64-.  Ok to install?

(Richi, is the non-optimization choice on ia32 something unexpected that
ought to be looked into?  I could file a PR, and maybe even look into it
a bit further.)


for  gcc/testsuite/ChangeLog

	* gcc.dg/tree-ssa/scev-3.c: xfail on ia32.
	* gcc.dg/tree-ssa/scev-5.c: Likewise.

Issue: gcc#155
TN: W517-007
---
 gcc/testsuite/gcc.dg/tree-ssa/scev-3.c |    2 +-
 gcc/testsuite/gcc.dg/tree-ssa/scev-5.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Richard Biener Nov. 9, 2023, 7:33 a.m. UTC | #1 
On Wed, 8 Nov 2023, Alexandre Oliva wrote:

> 
> These gimplefe tests never got the desired optimization on ia32, but
> they only started visibly failing when the representation of MEMs in
> dumps changed from printing 'symbol: a' to '&a'.
> 
> The transformation is not considered profitable on ia32, that's why it
> doesn't take place.  Maybe that's a bug in itself, but it's not a
> regression, and not something to be noisy about.
> 
> Regstrapped on x86_64-linux-gnu, also tested with gcc-13 on i686- and
> x86_64-.  Ok to install?

OK.

> (Richi, is the non-optimization choice on ia32 something unexpected that
> ought to be looked into?  I could file a PR, and maybe even look into it
> a bit further.)

There might be even a PR already.  The testcase expects that IVOPTs
chooses an IV that satisfies both

 a_p = &a[i_12];

and

 *&a[i_12] = 100;

basically code-generating a LEA, a store of the address and an
register indirect memory access.  That's what happens for 64bit
(and presumably on all other archs).  For some reason (I can only
guess costing), on ia32 we choose to prioritize using a single
induction variable (we need the original GIV for the exit test)
and so we get an obfuscated LEA for the address store and a
base with scaled index access for the store.

Note the testcase is a bit "bad" because we later sink the store
to a_p, so the generated assembly for the ia32 looks actually better.

Richard.


> 
> for  gcc/testsuite/ChangeLog
> 
> 	* gcc.dg/tree-ssa/scev-3.c: xfail on ia32.
> 	* gcc.dg/tree-ssa/scev-5.c: Likewise.
> 
> Issue: gcc#155
> TN: W517-007
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/scev-3.c |    2 +-
>  gcc/testsuite/gcc.dg/tree-ssa/scev-5.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
> index 4babd33f5c062..ac8c8d4519e30 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
> @@ -40,4 +40,4 @@ __BB(6):
>  
>  }
>  
> -/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" } } */
> +/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" { xfail ia32 } } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c b/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
> index c2feebdfc2489..c911a9298866f 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
> @@ -40,4 +40,4 @@ __BB(6):
>  
>  }
>  
> -/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" } } */
> +/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" { xfail ia32 } } } */
> 
>
Thomas Schwinge Nov. 11, 2023, 2:49 p.m. UTC | #2 
Hi!

On 2023-11-08T13:01:47-0300, Alexandre Oliva <oliva@adacore.com> wrote:
> These gimplefe tests never got the desired optimization on ia32, but
> they only started visibly failing when the representation of MEMs in
> dumps changed from printing 'symbol: a' to '&a'.

ACK -- but why not likewise "fix" the 'gcc.dg/tree-ssa/scev-4.c' FAIL?

    PASS: gcc.dg/tree-ssa/scev-3.c (test for excess errors)
    [-FAIL:-]{+XFAIL:+} gcc.dg/tree-ssa/scev-3.c scan-tree-dump-times ivopts "&a" 1
    PASS: gcc.dg/tree-ssa/scev-4.c (test for excess errors)
    FAIL: gcc.dg/tree-ssa/scev-4.c scan-tree-dump-times ivopts "&a" 1
    PASS: gcc.dg/tree-ssa/scev-5.c (test for excess errors)
    [-FAIL:-]{+XFAIL:+} gcc.dg/tree-ssa/scev-5.c scan-tree-dump-times ivopts "&a" 1


Grüße
 Thomas


> The transformation is not considered profitable on ia32, that's why it
> doesn't take place.  Maybe that's a bug in itself, but it's not a
> regression, and not something to be noisy about.
>
> Regstrapped on x86_64-linux-gnu, also tested with gcc-13 on i686- and
> x86_64-.  Ok to install?
>
> (Richi, is the non-optimization choice on ia32 something unexpected that
> ought to be looked into?  I could file a PR, and maybe even look into it
> a bit further.)
>
>
> for  gcc/testsuite/ChangeLog
>
>       * gcc.dg/tree-ssa/scev-3.c: xfail on ia32.
>       * gcc.dg/tree-ssa/scev-5.c: Likewise.
>
> Issue: gcc#155
> TN: W517-007
> ---
>  gcc/testsuite/gcc.dg/tree-ssa/scev-3.c |    2 +-
>  gcc/testsuite/gcc.dg/tree-ssa/scev-5.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
> index 4babd33f5c062..ac8c8d4519e30 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
> @@ -40,4 +40,4 @@ __BB(6):
>
>  }
>
> -/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" } } */
> +/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" { xfail ia32 } } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c b/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
> index c2feebdfc2489..c911a9298866f 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
> @@ -40,4 +40,4 @@ __BB(6):
>
>  }
>
> -/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" } } */
> +/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" { xfail ia32 } } } */
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Alexandre Oliva Nov. 13, 2023, 8:30 p.m. UTC | #3 
On Nov 11, 2023, Thomas Schwinge <thomas@codesourcery.com> wrote:

> ACK -- but why not likewise "fix" the 'gcc.dg/tree-ssa/scev-4.c' FAIL?

I have evidence from earlier compiler version bumps that there's some
correlation and that scev-4.c also failed in the past, but it wasn't
failing for me this time.
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
index 4babd33f5c062..ac8c8d4519e30 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-3.c
@@ -40,4 +40,4 @@  __BB(6):
 
 }
 
-/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" } } */
+/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" { xfail ia32 } } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c b/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
index c2feebdfc2489..c911a9298866f 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/scev-5.c
@@ -40,4 +40,4 @@  __BB(6):
 
 }
 
-/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" } } */
+/* { dg-final { scan-tree-dump-times "&a" 1 "ivopts" { xfail ia32 } } } */