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

[Evan Cheng]

I don't have a problem with the patch. But is there a follow up patch to fix the lowering problem?

Looking back at the history. It mentioned this impacts a couple of small test cases:

int
blah(_Bool x, _Bool y) {
  return (x - y) + 1;
}

int
blah2(bool x) {
  return x ? 0 : -1;
}

BTW, David filed the bug report that motivated the patch.

Evan

On Jul 19, 2013, at 2:10 PM, Nick Lewycky <nlewycky at google.com> wrote:

> 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/2b42891e/attachment.html>