[PATCH] Transform C - zext(bool) -> bool ? C - 1 : C

Nick Lewycky nlewycky at google.com
Fri Jul 19 14:10:49 PDT 2013


Hi Evan, this is about your r159230,
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120625/145470.html.
The canonicalization was added, then removed by you. In post-commit
review, Eli asked you to pick one of the two forms to be canonical.

I have no problem with David landing this patch as is -- it looks like
goodness to me, particularly the "C - zext(X)" transform -- but I don't
want to mess you up. Could you please chime in?

Nick


On 12 July 2013 16:03, David Majnemer <david.majnemer at gmail.com> wrote:

> The attached patch essentially reverts r159230.
>
> The reasons why are because:
> 1. It creates an asymmetry in the IR by hurting our ability to
> canonicalize things like C - zext(X) and C + zext(!X) into the same thing.
> 2. Adding it back does not break any tests and without it we still
> generate the inferior code mentioned in the commit log.
>
> It seems like we should keep the canonicalization and fix (the apparently
> still broken problem) in the lowering stage.
> The commit mentioned that some targets don't have a sext i1. This
> shouldn't be a concern at the IR level, should it?
>
> --
> David Majnemer
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130719/402db51d/attachment.html>


More information about the llvm-commits mailing list