Re: [PATCH] Fix PR 53395: tree-if-conv.c not producing MAX_EXPR

[Richard Guenther <richard dot guenther at gmail dot com>]

On Tue, Aug 28, 2012 at 9:09 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Mon, May 21, 2012 at 2:56 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Sun, May 20, 2012 at 1:40 AM, Andrew Pinski <pinskia@gmail.com> wrote:
>>> The problem here is that tree-if-conv.c produces COND_EXPR instead of
>>> the MAX/MIN EXPRs.  When I added the expansion from COND_EXPR to
>>> conditional moves, I assumes that the expressions which should have
>>> been converted into MAX_EXPR/MIN_EXPR have already happened.
>>>
>>> This fixes the problem by having tree-if-conv fold the expression so
>>> the MIN_EXPR/MAX_EXPR appears in the IR rather than COND_EXPR and the
>>> expansion happens correctly to the min/max rtl rather than just
>>> through the conditional move ones.
>>>
>>> OK?  Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>>
>> As we are unconditionally building a gimple_assign from the folding result
>> you need to re-gimplify it.  The code was using build3 instead of fold_build3
>> to avoid that and to make sure we create a plain COND_EXPR we can
>> later CSE / simplify properly.  Generating a MAX_EXPR directly is certainly
>> fine (can we always vectorize that?), but I suppose simply changing the
>> code to use fold_build3 will have unwanted fallout (consider folds habit
>> to insert conversions that are not requried on gimple).
>>
>> So I'd rather prefer to abstract the build3 (COND_EXPR,... into a
>> helper function that uses fold_ternary and only if the result is an
>> invariant/register or a MIN/MAX_EXPR use the result, canonicalizing
>> it properly.
>
> Here is an updated patch which does exactly that.
>
> Note I noticed another regression dealing with my expansion for
> COND_EXPR dealing with min/max and I will be submitting patches for
> those issue tomorrow.
>
> OK? Bootstrap and tested on x86_64-linux-gnu.

Ok.

Thanks,
Richard.

> Thanks,
> Andrew Pinski
>
> ChangeLog:
> * tree-if-conv.c (constant_or_ssa_name): New function.
> (fold_build_cond_expr): New function.
> (predicate_scalar_phi): Use fold_build_cond_expr instead of build3.
> (predicate_mem_writes): Likewise.