On Tue, 5 Mar 2019, Richard Biener wrote: > 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?
Ah - beacause the testcase from r255510 would break... The above is still a "bit" very tied to how we fold memcpy and friends, thus unreliable. Isn't the issue for the memcpy thing that we fail to record an access for the char[] access (remember it's now char[] MEM_REF, no longer struct X * MEM_REF with ref-all!). So maybe it now _does_ work with just return false... Richard.
