[PATCH] Do not transform PHI to select if doing so would be unsafe

[David Majnemer]

On Monday, June 3, 2013, Rafael Espíndola wrote:

> On 2 June 2013 04:08, David Majnemer <david.majnemer at gmail.com<javascript:;>>
> wrote:
> > PR16069 is an interesting case where an incoming value to a PHI is a trap
> > value while also being a 'ConstantExpr'.
> >
> > We do not consider this case when performing the 'HoistThenElseCodeToIf'
> > optimization.
> >
> > The attached patch will make our modifications more conservative if we
> > detect that we cannot transform the PHI to a select.
>
> The change to isValueEqualityComparison is just a cleanup, right? If
> so, please commit that first as an independent patch.


Will do.


>
> The comment
>   // If we get here, we can hoist at least one instruction
>
> is now out of date.


Fair, I'll clean that up.


>
> Why do you need the isa<ConstantExpr>? Can't the check be just
>
>      if (!isSafeToSpeculativelyExecute(BB1V) ||
> !isSafeToSpeculativelyExecute(BB2V))
>        return Changed;


The reason why is for performance and correctness. I don't want to look at
every instruction that can be fed into an incoming value for a phi. I just
want to see the ones that can be executed *before* the phi node in the same
BB as the phi. Which are these? ConstantExprs. FWIW, another part of
SimplifyCFG operates in the exact same way.


>
> LGTM with those changes.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130603/b4fe9ff2/attachment.html>