[Bug tree-optimization/62217] [4.9/5 Regression] DOM confuses complete unrolling which in turn causes VRP to warn

["law at redhat dot com" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62217

--- Comment #13 from Jeffrey A. Law <law at redhat dot com> ---
On 02/17/15 02:44, rguenther at suse dot de wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62217
>
> --- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
> On Mon, 16 Feb 2015, law at redhat dot com wrote:
>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62217
>>
>> --- Comment #9 from Jeffrey A. Law <law at redhat dot com> --- Yes, any
>> particular choice has the potential to regress in one way or another.
>> These are heuristics after all.  We're just looking for a reasonable
>> refinement if we can find one.
>
> Well, there is also canonicalization for CSE / jump threading.  Consider
>
> if (i == 2)
>   {
>     if (i != j)
>       ...
>     else if (j == 2)
>       ...
>
> or
>
> y = 2 * i;
> if (i == j)
>    x = i + j;
>
> but yes, this is followup transforms.  Unfortunately(?) DOM performs
> copy/constant propagation for all recorded equalities.
Yea, I've been mulling what it would take to instead build equivalence 
classes, then when we find a use walk through the class and see which 
one is "best".  I suspect the vast majority of the time the "best" is 
always going to be the same -- the major exception will be these 
temporary equivalences.   I've also been pondering somehow marking the 
equivalent values with some kind of tag indicating their scope and using 
that to guide propagation.

But I haven't prototyped anything...


>
> As you say, we don't know - we only know that properly canonicalizing
> will expose the followup transforms if there are any.  It looks like
> we are basically taking the original order of EQ_EXPR operands
> (thus eventually rely on tree_swap_operands canonicalization) plus
> the "correctness" thing of taking into account loop depth (which is
> kind of a dominance relation).
Correct, we take it from the original order and thus from the earlier 
canonicalization via tree_swap_operands.

>
> We are also introducing SSA_NAME_VALUE "chains" in record_equality
> as we are setting x SSA_NAME_VALUE to y not to SSA_NAME_VALUE (y)
> (we seem to do this in multiple places for some odd reason...,
> only tree-ssa-threadedge.c:record_temporary_equivalence seems to get
> this right).
Even if we set to SSA_NAME_VALUE (y) we can end up with chains.  But 
this change might eliminate the benefit of the chain walk, which would 
be good.

Jeff