[PR 85762, 87008, 85459] Relax MEM_REF check in contains_vce_or_bfcref_p

Richard Biener rguenther@suse.de
Wed Apr 17 10:03:00 GMT 2019


On Wed, 17 Apr 2019, Martin Jambor wrote:

> Hello,
> 
> On Sun, Mar 10 2019, Martin Jambor wrote:
> > Hi,
> >
> > after we have accidentally dropped the mailing list from our discussion
> > (my apologies for not spotting that in time), Richi has approved the
> > following patch which I have bootstrapped and tested on x86_64-linux
> > (all languages) and on i686-linux, aarch64-linux and ppc64-linux (C, C++
> > and Fortran) and so I am about to commit it to trunk.
> >
> > It XFAILS three guality tests which pass at -O0, which means there are
> > three additional XPASSes - there already are 5 pre-existing XPASSes in
> > that testcase and 29 outright failures.  I will come back to this next
> > in April and see whether I can make the tests pass by decoupling the
> > roles now played by cannot_scalarize_away_bitmap (or at least massage
> > the testcase to go make the XPASSes go away).  But I won't have time to
> > do it next two weeks and this patch is important enough to have it in
> > trunk now.  I intend to backport it to gcc 8 in April too.
> >
> > Thanks,
> >
> > Martin
> >
> >
> > 2019-03-08  Martin Jambor  <mjambor@suse.cz>
> >
> > 	PR tree-optimization/85762
> > 	PR tree-optimization/87008
> > 	PR tree-optimization/85459
> > 	* tree-sra.c (contains_vce_or_bfcref_p): New parameter, set the bool
> > 	it points to if there is a type changing MEM_REF.  Adjust all callers.
> > 	(build_accesses_from_assign): Disable total scalarization if
> > 	contains_vce_or_bfcref_p returns true through the new parameter, for
> > 	both rhs and lhs.
> >
> > 	testsuite/
> > 	* g++.dg/tree-ssa/pr87008.C: New test.
> > 	* gcc.dg/guality/pr54970.c: Xfail tests querying a[0] everywhere.
> 
> this patch has been on trunk for over a month and at least so far nobody
> complained.  I have applied it to gcc-8-branch and did a bootstrap and
> testing on an x86_64-linux machine and there were no problems either.
> 
> Therefore I would propose to backport it - the other option being leaving
> the gcc 8 regression(s) unfixed.  What do you think?

Let's go for the backport.

Richard.

> Martin
> 
> 
> 2019-04-16  Martin Jambor  <mjambor@suse.cz>
> 
> 	Backport from mainline
> 	2019-03-10  Martin Jambor  <mjambor@suse.cz>
> 
>         PR tree-optimization/85762
>         PR tree-optimization/87008
>         PR tree-optimization/85459
>         * tree-sra.c (contains_vce_or_bfcref_p): New parameter, set the bool
>         it points to if there is a type changing MEM_REF.  Adjust all callers.
>         (build_accesses_from_assign): Disable total scalarization if
>         contains_vce_or_bfcref_p returns true through the new parameter, for
>         both rhs and lhs.
> 
>         testsuite/
>         * g++.dg/tree-ssa/pr87008.C: New test.
>         * gcc.dg/guality/pr54970.c: Xfail tests querying a[0] everywhere.
> ---
>  gcc/testsuite/g++.dg/tree-ssa/pr87008.C | 17 ++++++++++++
>  gcc/testsuite/gcc.dg/guality/pr54970.c  |  6 ++---
>  gcc/tree-sra.c                          | 36 ++++++++++++++++++-------
>  3 files changed, 47 insertions(+), 12 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/tree-ssa/pr87008.C
> 
> diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr87008.C b/gcc/testsuite/g++.dg/tree-ssa/pr87008.C
> new file mode 100644
> index 00000000000..eef521f9ad5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/tree-ssa/pr87008.C
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> +
> +extern void dontcallthis();
> +
> +struct A { long a, b; };
> +struct B : A {};
> +template<class T>void cp(T&a,T const&b){a=b;}
> +long f(B x){
> +  B y; cp<A>(y,x);
> +  B z; cp<A>(z,x);
> +  if (y.a - z.a)
> +    dontcallthis ();
> +  return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump-not "dontcallthis" "optimized" } } */
> diff --git a/gcc/testsuite/gcc.dg/guality/pr54970.c b/gcc/testsuite/gcc.dg/guality/pr54970.c
> index 1819d023e21..f12a9aac1d2 100644
> --- a/gcc/testsuite/gcc.dg/guality/pr54970.c
> +++ b/gcc/testsuite/gcc.dg/guality/pr54970.c
> @@ -8,17 +8,17 @@
>  int
>  main ()
>  {
> -  int a[] = { 1, 2, 3 };	/* { dg-final { gdb-test 15 "a\[0\]" "1" } } */
> +  int a[] = { 1, 2, 3 };	/* { dg-final { gdb-test 15 "a\[0\]" "1" { xfail { *-*-* } } } } */
>    int *p = a + 2;		/* { dg-final { gdb-test 15 "a\[1\]" "2" } } */
>    int *q = a + 1;		/* { dg-final { gdb-test 15 "a\[2\]" "3" } } */
>  				/* { dg-final { gdb-test 15 "*p" "3" } } */
>    asm volatile (NOP);		/* { dg-final { gdb-test 15 "*q" "2" } } */
> -  *p += 10;			/* { dg-final { gdb-test 20 "a\[0\]" "1" } } */
> +  *p += 10;			/* { dg-final { gdb-test 20 "a\[0\]" "1" { xfail { *-*-* } } } } */
>  				/* { dg-final { gdb-test 20 "a\[1\]" "2" } } */
>  				/* { dg-final { gdb-test 20 "a\[2\]" "13" } } */
>  				/* { dg-final { gdb-test 20 "*p" "13" } } */
>    asm volatile (NOP);		/* { dg-final { gdb-test 20 "*q" "2" } } */
> -  *q += 10;			/* { dg-final { gdb-test 25 "a\[0\]" "1" } } */
> +  *q += 10;			/* { dg-final { gdb-test 25 "a\[0\]" "1" { xfail { *-*-* } } } } */
>  				/* { dg-final { gdb-test 25 "a\[1\]" "12" } } */
>  				/* { dg-final { gdb-test 25 "a\[2\]" "13" } } */
>  				/* { dg-final { gdb-test 25 "*p" "13" } } */
> diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
> index bb373b33b7a..e1ebdfaa225 100644
> --- a/gcc/tree-sra.c
> +++ b/gcc/tree-sra.c
> @@ -1150,29 +1150,36 @@ contains_view_convert_expr_p (const_tree ref)
>    return false;
>  }
>  
> -/* Return true if REF contains a VIEW_CONVERT_EXPR or a MEM_REF that performs
> -   type conversion or a COMPONENT_REF with a bit-field field declaration.  */
> +/* Return true if REF contains a VIEW_CONVERT_EXPR or a COMPONENT_REF with a
> +   bit-field field declaration.  If TYPE_CHANGING_P is non-NULL, set the bool
> +   it points to will be set if REF contains any of the above or a MEM_REF
> +   expression that effectively performs type conversion.  */
>  
>  static bool
> -contains_vce_or_bfcref_p (const_tree ref)
> +contains_vce_or_bfcref_p (const_tree ref, bool *type_changing_p = NULL)
>  {
>    while (handled_component_p (ref))
>      {
>        if (TREE_CODE (ref) == VIEW_CONVERT_EXPR
>  	  || (TREE_CODE (ref) == COMPONENT_REF
>  	      && DECL_BIT_FIELD (TREE_OPERAND (ref, 1))))
> -	return true;
> +	{
> +	  if (type_changing_p)
> +	    *type_changing_p = true;
> +	  return true;
> +	}
>        ref = TREE_OPERAND (ref, 0);
>      }
>  
> -  if (TREE_CODE (ref) != MEM_REF
> +  if (!type_changing_p
> +      || TREE_CODE (ref) != MEM_REF
>        || TREE_CODE (TREE_OPERAND (ref, 0)) != ADDR_EXPR)
>      return false;
>  
>    tree mem = TREE_OPERAND (TREE_OPERAND (ref, 0), 0);
>    if (TYPE_MAIN_VARIANT (TREE_TYPE (ref))
>        != TYPE_MAIN_VARIANT (TREE_TYPE (mem)))
> -    return true;
> +    *type_changing_p = true;
>  
>    return false;
>  }
> @@ -1368,15 +1375,26 @@ build_accesses_from_assign (gimple *stmt)
>        lacc->grp_assignment_write = 1;
>        if (storage_order_barrier_p (rhs))
>  	lacc->grp_unscalarizable_region = 1;
> +
> +      if (should_scalarize_away_bitmap && !is_gimple_reg_type (lacc->type))
> +	{
> +	  bool type_changing_p = false;
> +	  contains_vce_or_bfcref_p (lhs, &type_changing_p);
> +	  if (type_changing_p)
> +	    bitmap_set_bit (cannot_scalarize_away_bitmap,
> +			    DECL_UID (lacc->base));
> +	}
>      }
>  
>    if (racc)
>      {
>        racc->grp_assignment_read = 1;
> -      if (should_scalarize_away_bitmap && !gimple_has_volatile_ops (stmt)
> -	  && !is_gimple_reg_type (racc->type))
> +      if (should_scalarize_away_bitmap && !is_gimple_reg_type (racc->type))
>  	{
> -	  if (contains_vce_or_bfcref_p (rhs))
> +	  bool type_changing_p = false;
> +	  contains_vce_or_bfcref_p (rhs, &type_changing_p);
> +
> +	  if (type_changing_p || gimple_has_volatile_ops (stmt))
>  	    bitmap_set_bit (cannot_scalarize_away_bitmap,
>  			    DECL_UID (racc->base));
>  	  else
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NÌrnberg)


More information about the Gcc-patches mailing list