[PATCH] D44367: [InstCombine] peek through FP casts for sign-bit compares (PR36682)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D44367#1034211, @lebedev.ri wrote:

> In https://reviews.llvm.org/D44367#1034208, @spatel wrote:
>
> > In https://reviews.llvm.org/D44367#1034194, @lebedev.ri wrote:
> >
> > > In the mean time i experimented with generating the testcases (clearly, overdone it), and this patch does not handle all the cases i've come up with. Or maybe the cases are just invalid, which is more likely.
> >
> >
> > Nice. I thought about the X == 0 and X != 0 cases just after I posted this. I think those are safe to add in a small follow-up patch.
> >
> > We should canonicalize other signbit-check predicates (sle / sge) to this form. Are you seeing any of those cases fail to match with this patch?
>
>
> Eh, hard to say, there are more not changed patterns than changed, as you can see from the attached `cast-int-icmp-eq-0.ll`.
>  In particular, if the sizes change (when input size != bitcast fp type), then it's not handled.


The tests in this patch have size-changing casts (both larger and smaller than the original type). Did I miss something?

> All `uitofp` are not handled, obviously. And non `slt 0`/`sgt -1` condition codes are not touched, too.

I think uitofp followed by a signbit check will be optimized away completely to true/false, so that can go in InstSimplify. That's another small, independent patch.

> Should some version of that  `cast-int-icmp-eq-0.ll` be committed? (anything missing from it?)

Sure - if you want to check it in, I can rebase this patch to show the diffs. Including some weird FP types like PPC_FP128 might be interesting. And having some 'undef' in vector tests as I have here also increases coverage. 
But I don't see much value in repeating every test for every combination of types, sle/sge, or vector+scalar repeats of everything (if we know the transform works for at least one vector type, then it almost certainly works in the other cases).


https://reviews.llvm.org/D44367