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 <mjam...@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 <mjam...@suse.cz>
>
> Backport from mainline
> 2019-03-10 Martin Jambor <mjam...@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 <rguent...@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
