[PATCH] D44421: [InstSimplify] [NFC] Add tests for peeking through unsigned FP casts for sign compares (PR36682)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/trunk/test/Transforms/InstSimplify/cast-unsigned-icmp-cmp-0.ll:2-5
+; RUN: opt < %s -instcombine -instsimplify -S | FileCheck %s
+
+; NOTE: we run instcombine first,
+; to make sure that it does *NOT* drop uitofp+bitcast here.
----------------
spatel wrote:
> lebedev.ri wrote:
> > lebedev.ri wrote:
> > > spatel wrote:
> > > > Sorry - I didn't catch this before. Why are we doing anything with -instcombine here? We should only be running -instsimplify on this file.
> > > You commented on the explanation note :)
> > > )I moved it here from the next diff before commit, should have done that earlier)
> > Uhm, yeah, i was thinking of something else, that instcombine transform would not be valid anyway:
> > ```
> > $ ~/src/alive-nj/run.py /tmp/slt.txt 
> > ----------                                                                      
> > Name: slt
> >   %f   = uitofp i32 %i to float
> >   %b   = bitcast float %f to i32
> >   %cmp = icmp slt i32 %b, 0
> > =>
> >   %cmp = icmp slt i32 %i, 0
> > 
> > ERROR: Mismatch in values for i1 %cmp                                           
> > 
> > Example:
> >   i32 %i = 0x80000000 (2147483648, -2147483648)
> > float %f = 1*(2**31)
> >   i32 %b = 0x0f800000 (260046848)
> > source: 0x0 (0)
> > target: 0x1 (1, -1)
> > 
> > $ ~/src/alive-nj/run.py /tmp/sgt.txt
> > ----------                                                                      
> > Name: sgt
> >   %f   = uitofp i32 %i to float
> >   %b   = bitcast float %f to i32
> >   %cmp = icmp sgt i32 %b, -1
> > =>
> >   %cmp = icmp sgt i32 %i, -1
> > 
> > ERROR: Mismatch in values for i1 %cmp                                           
> > 
> > Example:
> >   i32 %i = 0x80000000 (2147483648, -2147483648)
> > float %f = 1*(2**31)
> >   i32 %b = 0x0f800000 (260046848)
> > source: 0x1 (1, -1)
> > target: 0x0 (0)
> > 
> > ```
> > 
> > Sorry, let me remove that pass..
> I'm confused. We're going to add code to -instsimplify, so these tests should be only for that pass. Why is -instcombine in the RUN line?
```
; NOTE: we run instcombine first,
; to make sure that it does *NOT* drop uitofp+bitcast here.
```
In other words, i *thought* that running `instcombine` first would make sure that
a combine to drop uitofp+bitcast from that case won't be added to instcombine.
If it would be added, then we wouldn't be able to simplify to a boolean here.

But such a fold would be invalid, so there is zero point in running `instcombine`
in this case, thus i will now drop that pass from the runline.


Repository:
  rL LLVM

https://reviews.llvm.org/D44421