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

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #12)
> Created attachment 59781 [details]
> Possible patch
> 
> I have had a look at this PR and can propose the attached patch that seems
> to fix the issue.  It does so by creating new variables during phi
> translation of expression whose variables are not defined in the current
> block.  I suppose there could be an alternative fix that would avoid the
> creation of new variables, and just reset the flow sensitive information of
> existing variables when using them outside of their definition scope.  I
> didn't try it though, and I won't have time to study this further, so I'm
> posting the patch so that it's not lost.

The patch basically papers over the issue that we try to do PHI translation
on the expression representation to keep those "valid" but take a detour
through valueization.  Translating the expression operand newnary->op[i] we
do

                pre_expr leader, result;
                unsigned int op_val_id = VN_INFO (newnary->op[i])->value_id;
                leader = find_leader_in_sets (op_val_id, set1, set2);
                result = phi_translate (dest, leader, set1, set2, e);
                if (result && result != leader)
                  /* If op has a leader in the sets we translate make
                     sure to use the value of the translated expression.
                     We might need a new representative for that.  */
                  newnary->op[i] = get_representative_for (result, pred);
                else if (!result)
                  return NULL;

                changed |= newnary->op[i] != nary->op[i];

so we turn the op into a value and then look for a leader in our set
of expressions to translate (but not necessarily prefering nary->op[i]
itself).  But we only skip get_representative_for if phi-translation
did anything - the patch always assigns a new representative.
To that extent the original newnary->op[i] was already "bad" (but 
as indicated in comment#8 I believed that was all OK?).

That said, I know the transition to making expressions first class is done
only half-way.  Expressions in the sets should always be "representatives"
at the respective program point to be eligible for expression simplification.

One could also try to attack the issue by making sure all expressions for
the same value we'd insert for would behave the same before re-using the
value for the inserted PHI.  But somehow that also feels odd.

Reply via email to