diff mbox

Fix Ada testsuite failures introduced by my folding patch

Message ID 20100905141956.GH31380@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Sept. 5, 2010, 2:19 p.m. UTC
Hi,
my previous folding patch introduced several Ada testsuite failures.  In some cases
we replace a=b by a={1,2} that is no longer valid gimple.  Other case is:

.3194_2 = (void (*ada__tags__prim_ptr) (void)[1:1] * {ref-all}) &c390011_2__tableT + 32;

here I think the expression can be validized and is ommision in
maybe_fold_offset_to_address, but i did not put too much effort to figure out where it fails.

This patch adds test that result of fold_const_aggregate_ref is min_invariant
before using it.  The function can return constructors to allow self recursion
when handling multiple references.

In the first case disabling transform is the only correct thing to do. In the second case
this just turns ICE into suboptimal codegen.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* gimple-fold.c (maybe_fold_reference): Test that result of
	fold_const_aggregate_ref is gimple_min_invariant.

Comments

Gerald Pfeifer Sept. 5, 2010, 2:53 p.m. UTC | #1
On Sun, 5 Sep 2010, Jan Hubicka wrote:
> my previous folding patch introduced several Ada testsuite failures.

Is there a non-Ada testcase that you could add, Honza?

Gerald
Jan Hubicka Sept. 5, 2010, 3:16 p.m. UTC | #2
> On Sun, 5 Sep 2010, Jan Hubicka wrote:
> > my previous folding patch introduced several Ada testsuite failures.
> 
> Is there a non-Ada testcase that you could add, Honza?
no, I don't have any.  The testcases are array assignments, so having C equivalent
is tricky.  I tried version with struct and it does not trigger an ICE.

Honza
> 
> Gerald
Richard Biener Sept. 5, 2010, 5:22 p.m. UTC | #3
On Sun, 5 Sep 2010, Jan Hubicka wrote:

> Hi,
> my previous folding patch introduced several Ada testsuite failures.  In some cases
> we replace a=b by a={1,2} that is no longer valid gimple.  Other case is:
> 
> .3194_2 = (void (*ada__tags__prim_ptr) (void)[1:1] * {ref-all}) &c390011_2__tableT + 32;
> 
> here I think the expression can be validized and is ommision in
> maybe_fold_offset_to_address, but i did not put too much effort to figure out where it fails.
> 
> This patch adds test that result of fold_const_aggregate_ref is min_invariant
> before using it.  The function can return constructors to allow self recursion
> when handling multiple references.
> 
> In the first case disabling transform is the only correct thing to do. In the second case
> this just turns ICE into suboptimal codegen.
> 
> Bootstrapped/regtested x86_64-linux, OK?

Ok.

Thanks,
Richard.

> Honza
> 
> 	* gimple-fold.c (maybe_fold_reference): Test that result of
> 	fold_const_aggregate_ref is gimple_min_invariant.
> Index: gimple-fold.c
> ===================================================================
> --- gimple-fold.c	(revision 163862)
> +++ gimple-fold.c	(working copy)
> @@ -477,7 +477,8 @@ maybe_fold_reference (tree expr, bool is
>    tree result;
>  
>    if (!is_lhs
> -      && (result = fold_const_aggregate_ref (expr)))
> +      && (result = fold_const_aggregate_ref (expr))
> +      && is_gimple_min_invariant (result))
>      return result;
>  
>    /* ???  We might want to open-code the relevant remaining cases
> 
>
Eric Botcazou Sept. 7, 2010, 5:25 p.m. UTC | #4
> 	* gimple-fold.c (maybe_fold_reference): Test that result of
> 	fold_const_aggregate_ref is gimple_min_invariant.

Has it been applied?  There are still several (6) ACATS failures on x86-64:
  http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00635.html
and even more on SPARC64 for example.
Jan Hubicka Sept. 7, 2010, 9 p.m. UTC | #5
> > 	* gimple-fold.c (maybe_fold_reference): Test that result of
> > 	fold_const_aggregate_ref is gimple_min_invariant.
> 
> Has it been applied?  There are still several (6) ACATS failures on x86-64:
>   http://gcc.gnu.org/ml/gcc-testresults/2010-09/msg00635.html
> and even more on SPARC64 for example.

I applied it today, so it should be gone now.  I will double check.

Honza
> 
> -- 
> Eric Botcazou
diff mbox

Patch

Index: gimple-fold.c
===================================================================
--- gimple-fold.c	(revision 163862)
+++ gimple-fold.c	(working copy)
@@ -477,7 +477,8 @@  maybe_fold_reference (tree expr, bool is
   tree result;
 
   if (!is_lhs
-      && (result = fold_const_aggregate_ref (expr)))
+      && (result = fold_const_aggregate_ref (expr))
+      && is_gimple_min_invariant (result))
     return result;
 
   /* ???  We might want to open-code the relevant remaining cases