On Tue, 5 Mar 2019, Martin Jambor wrote: > Hi, > > after looking into the three PRs I found that the problem is the same in > all of them, introduced in revision 255510, with SRA treating completely > non type-punning MEM_REFs to a filed of a structure as a V_C_E (these > are typically introduced by inlining tiny C++ getters/setters and other > methods), sending it to a paranoid mode and leaving many unnecessary > memory accesses behind. > > I believe the right fix is to relax the condition to what it was > intended to do in r255510 to fix PR 83141. This particular behavior was > chosen because we were afraid floating-point accesses might be > introduced to load non-FP data, but as https://pastebin.com/Jk7ZPsVH > shows, that can still happen and so if it really must be avoided at all > costs, we have to deal with it differently. > > The regressions this fixes are potentially severe, therefore I'd like to > backport this also to the gcc-8-branch. The patch has passed bootstrap > and testing on x86_64-linux and aarch64-linux, testing and bootstrap on > i686-linux and ppc64le-linux are in progress. > > OK for trunk and then later on for the branch? > > Thanks, > > > Martin > > > 2019-03-01 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): Relax MEM_REF type-conversion > check. > > testsuite/ > * g++.dg/tree-ssa/pr87008.C: New test. > --- > gcc/testsuite/g++.dg/tree-ssa/pr87008.C | 17 +++++++++++++++++ > gcc/tree-sra.c | 13 ++++--------- > 2 files changed, 21 insertions(+), 9 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/tree-sra.c b/gcc/tree-sra.c > index eeef31ba496..bd30af5c6e0 100644 > --- a/gcc/tree-sra.c > +++ b/gcc/tree-sra.c > @@ -1151,7 +1151,7 @@ contains_view_convert_expr_p (const_tree ref) > } > > /* 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. */ > + memcpy or a COMPONENT_REF with a bit-field field declaration. */ > > static bool > contains_vce_or_bfcref_p (const_tree ref) > @@ -1165,14 +1165,9 @@ contains_vce_or_bfcref_p (const_tree ref) > ref = TREE_OPERAND (ref, 0); > } > > - if (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; > + if (TREE_CODE (ref) == MEM_REF > + && TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (TREE_OPERAND (ref, 1)))) > + return true;
This doesn't make much sense to me - why not simply go back to the old implementation which would mean to just return false; here? Richard.