On Wed, 12 Jun 2013, Richard Biener wrote:
On Wed, Jun 12, 2013 at 10:47 AM, Marc Glisse <marc.gli...@inria.fr> wrote:Essentially never. I tried with the fold_stmt version of the patch, and libstdc++-v3/src/c++98/concept-inst.cc is the only file where it triggers. Note that the case: b=*a *a=b is already handled by FRE which removes *a=b (copyprop later removes the dead b=*a and release_ssa removes the unused variable b), it is only *a=*a that wasn't handled. Now that I look at it, it is a bit surprising that: struct A {int i;}; void f(A*a,A*b){ A c=*b; *a=c; } void g(A*a,A*b){ *a=*b; } gives 2 different .optimized gimple: c$i_5 = MEM[(const struct A &)b_2(D)]; MEM[(struct A *)a_3(D)] = c$i_5; and: *a_2(D) = MEM[(const struct A &)b_3(D)]; Aren't they equivalent? And if so, which form should be preferred?Well, the first is optimized by SRA to copy element-wise and thus the loads/stores have is_gimple_reg_type () which require separate loads/stores. The second is an aggregate copy where we cannot generate SSA temporaries for the result of the load (!is_gimple_reg_type ()) and thus we are required to have a single statement. One of my pending GIMPLE re-org tasks is to always separate loads and stores and allow SSA names of aggregate type, thus we'd have tem_1 = MEM[(const struct A &)b_3(D)]; *a_2(D) = tem_1; even for the 2nd case. That solves the fact that we are missing an aggregate copy propagation pass quite nicely. Yes, you have to watch for not creating (too many) overlapping life-ranges as out-of-SSA won't be able to assign the temporary aggregate SSA names to registers but possibly has to allocate costly stack space for them. Setting SSA_NAME_OCCURS_IN_ABNORMAL_PHI on them solves this issue in a hacky way. On my list since about 5 years ... ;)
I was going to ask if I should wait for it (it makes my patch unnecessary), but I guess that answers it. Since Jeff seems ok, I committed it at r200034.
-- Marc Glisse
