[Bug tree-optimization/87008] [8/9 Regression] gimple mem-to-mem assignment badly optimized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87008 Jakub Jelinek changed: What|Removed |Added Target Milestone|8.3 |8.4 --- Comment #6 from Jakub Jelinek --- GCC 8.3 has been released.
[Bug tree-optimization/87008] [8/9 Regression] gimple mem-to-mem assignment badly optimized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87008 --- Comment #7 from Martin Jambor --- (In reply to Marc Glisse from comment #1) > struct A { double a, b; }; > struct B : A {}; > templatevoid cp(T&a,T const&b){a=b;} > double f(B x){ > B y; cp(y,x); > B z; cp(z,x); > return y.a - z.a; > } > > This is not quite equivalent because RTL manages to optimize this case, but > gimple, with -Ofast, still gets the ugly: > > ISRA.1 = MEM[(const struct A &)&x]; > SR.9_9 = MEM[(struct A *)&ISRA.1]; > ISRA.1 = MEM[(const struct A &)&x]; > SR.8_10 = MEM[(struct A *)&ISRA.1]; > _3 = SR.9_9 - SR.8_10; > return _3; > > Writing cp instead of cp makes it work, and the main difference starts > in SRA. I expect (didn't check) this is another victim of r255510 . Indeed, with the proof-of-concept patch from PR 85762, this gets optimized into: [local count: 1073741824]: SR.7_2 = MEM[(const struct A &)&x]; _3 = SR.7_2 - SR.7_2; return _3; with -O2 and to retrn 0 with -Ofast.
[Bug tree-optimization/87008] [8/9 Regression] gimple mem-to-mem assignment badly optimized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87008 --- Comment #8 from Martin Jambor --- Author: jamborm Date: Sun Mar 10 16:20:06 2019 New Revision: 269556 URL: https://gcc.gnu.org/viewcvs?rev=269556&root=gcc&view=rev Log: Make SRA less strict with memcpy performing MEM_REFs 2019-03-10 Martin Jambor 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. Added: trunk/gcc/testsuite/g++.dg/tree-ssa/pr87008.C Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/guality/pr54970.c trunk/gcc/tree-sra.c
[Bug tree-optimization/87008] [8/9 Regression] gimple mem-to-mem assignment badly optimized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87008 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #9 from Jakub Jelinek --- Is this now fixed on the trunk with r269556?
[Bug tree-optimization/87008] [8/9 Regression] gimple mem-to-mem assignment badly optimized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87008 Richard Biener changed: What|Removed |Added Priority|P3 |P2 --- Comment #5 from Richard Biener --- So on trunk ESRA does some pointless stuff at least: : ISRA.2 = MEM[(const struct A &)&x]; + SR.7_4 = MEM[(struct A *)&ISRA.2]; + SR.8_5 = MEM[(struct A *)&ISRA.2 + 8B]; + MEM[(struct A *)&ISRA.2] = SR.7_4; + MEM[(struct A *)&ISRA.2 + 8B] = SR.8_5; MEM[(struct A *)&y] = ISRA.2; - ISRA.2 ={v} {CLOBBER}; + y$D2304$a_17 = MEM[(struct A *)&y]; ISRA.2 = MEM[(const struct A &)&x]; + SR.5_20 = MEM[(struct A *)&ISRA.2]; + SR.6_21 = MEM[(struct A *)&ISRA.2 + 8B]; + MEM[(struct A *)&ISRA.2] = SR.5_20; + MEM[(struct A *)&ISRA.2 + 8B] = SR.6_21; MEM[(struct A *)&z] = ISRA.2; - ISRA.2 ={v} {CLOBBER}; - _1 = y.D.2304.a; - _2 = z.D.2304.a; + z$D2304$a_24 = MEM[(struct A *)&z]; + _1 = y$D2304$a_17; + _2 = z$D2304$a_24; _6 = _1 - _2; where the main grief is caused by IPA SRA which replaces cp (&y.D.2304, &x.D.2304); cp (&z.D.2304, &x.D.2304); _1 = y.D.2304.a; _2 = z.D.2304.a; _6 = _1 - _2; with _Z2cpI1AEvRT_RKS1_.isra.0 (&y, MEM[(const struct A &)&x]); _Z2cpI1AEvRT_RKS1_.isra.0 (&z, MEM[(const struct A &)&x]); _1 = y.D.2304.a; _2 = z.D.2304.a; _6 = _1 - _2; replacing the by-reference second argument by an aggregate by-value argument. I think that's unwarranted - Martin, can you see if there's a simple logic error that can rectify this? The same behavior is happening when the second parameter is not declared const. With IPA SRA disabled the IL gets nicer but then ESRA doesnt' do anything interesting anymore (but renaming stuff and uglifying/moving loads): MEM[(struct A *)&y] = MEM[(const struct A &)&x]; + y$D2304$a_4 = MEM[(struct A *)&y]; MEM[(struct A *)&z] = MEM[(const struct A &)&x]; - _1 = y.D.2304.a; - _2 = z.D.2304.a; + z$D2304$a_5 = MEM[(struct A *)&z]; + _1 = y$D2304$a_4; + _2 = z$D2304$a_5; _6 = _1 - _2; also sth to avoid IMHO. I guess it thinks it might fully scalarize the copies but late decides not to but leaves the rest of the trasform in-place. So yes, the copy is viewed as contains_vce_or_bfcref_p because we access a variable of type B via type A. But that only matters for variables we can scalarize away - we call this for MEM[&x] only but we want to scalarize MEM[&y] = MEM[&x] thus scalarize y away. That we mark x as cannot_scalarize_away_bitmap shouldn't affect total scalarization for the aggregate copy? I wonder why build_accesses_from_assign only looks at the RHS for total scalarization and not the LHS. Well to sum it up, we see all uses of y and z and thus we _can_ total scalarize y and z simply eliding the aggregate copy.
[Bug tree-optimization/87008] [8/9 Regression] gimple mem-to-mem assignment badly optimized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87008 --- Comment #1 from Marc Glisse --- struct A { double a, b; }; struct B : A {}; templatevoid cp(T&a,T const&b){a=b;} double f(B x){ B y; cp(y,x); B z; cp(z,x); return y.a - z.a; } This is not quite equivalent because RTL manages to optimize this case, but gimple, with -Ofast, still gets the ugly: ISRA.1 = MEM[(const struct A &)&x]; SR.9_9 = MEM[(struct A *)&ISRA.1]; ISRA.1 = MEM[(const struct A &)&x]; SR.8_10 = MEM[(struct A *)&ISRA.1]; _3 = SR.9_9 - SR.8_10; return _3; Writing cp instead of cp makes it work, and the main difference starts in SRA. I expect (didn't check) this is another victim of r255510 .
[Bug tree-optimization/87008] [8/9 Regression] gimple mem-to-mem assignment badly optimized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87008 --- Comment #2 from Marc Glisse --- Or just: struct A { double a, b; }; struct B : A {}; double f(B x){ B y; A*px=&x; A*py=&y; *py=*px; return y.a; } MEM[(struct A *)&y] = MEM[(const struct A &)&x]; y_6 = MEM[(struct A *)&y]; y ={v} {CLOBBER}; return y_6; where y_6 should be read directly from x. SRA doesn't dare touch it. SCCVN does see that reading from y is equivalent to reading from x, but unless something else is already reading from x, it keeps the read from y.
[Bug tree-optimization/87008] [8/9 Regression] gimple mem-to-mem assignment badly optimized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87008 Richard Biener changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2018-08-22 CC||jamborm at gcc dot gnu.org, ||rguenth at gcc dot gnu.org Target Milestone|--- |8.3 Ever confirmed|0 |1 --- Comment #3 from Richard Biener --- (In reply to Marc Glisse from comment #2) > Or just: > > struct A { double a, b; }; > struct B : A {}; > double f(B x){ > B y; > A*px=&x; > A*py=&y; > *py=*px; > return y.a; > } > > MEM[(struct A *)&y] = MEM[(const struct A &)&x]; > y_6 = MEM[(struct A *)&y]; > y ={v} {CLOBBER}; > return y_6; > > where y_6 should be read directly from x. SRA doesn't dare touch it. SCCVN > does see that reading from y is equivalent to reading from x, but unless > something else is already reading from x, it keeps the read from y. Yeah, SCCVN doesn't change the read to one from x because generally it cannot know y will go away and reading from x possibly enlarges its lifetime (no stack sharing). To really handle this we need to expose the way we'd expand such aggregate copies to RTL already at GIMPLE stage. SRA could be the pass that should eventually do that (but of course avoid exposing copy loops or calls to memcpy we might expand to ...). So it boils down to heuristics again...
[Bug tree-optimization/87008] [8/9 Regression] gimple mem-to-mem assignment badly optimized
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87008 --- Comment #4 from Richard Biener --- (In reply to Marc Glisse from comment #1) > struct A { double a, b; }; > struct B : A {}; > templatevoid cp(T&a,T const&b){a=b;} > double f(B x){ > B y; cp(y,x); > B z; cp(z,x); > return y.a - z.a; > } > > This is not quite equivalent because RTL manages to optimize this case, but > gimple, with -Ofast, still gets the ugly: > > ISRA.1 = MEM[(const struct A &)&x]; > SR.9_9 = MEM[(struct A *)&ISRA.1]; > ISRA.1 = MEM[(const struct A &)&x]; > SR.8_10 = MEM[(struct A *)&ISRA.1]; > _3 = SR.9_9 - SR.8_10; > return _3; > > Writing cp instead of cp makes it work, and the main difference starts > in SRA. I expect (didn't check) this is another victim of r255510 . The initial IL is too convoluted for early FRE to figure out the equivalences. For the above which is visible to the late FRE the issue is that the redundant aggregate copy gets in the way which isn't detected in early FRE and FRE also doesn't try to remove or detect redundant aggregate copies because we don't really "value-number" aggregate stores. To sum up it - aggregate copies are bad ;) But they also sometimes help - all the vn_reference_op_lookup_3 tricks wouldn't work without them unless you end up with store pieces that always fully cover all downstream loads.