[PATCH] D105632: [LV] Use lookThroughAnd with logical reductions

Fri Jul 9 08:08:43 PDT 2021

kmclaughlin added inline comments.

================
Comment at: llvm/test/Transforms/LoopVectorize/trunc-reductions.ll:186
+  %ext = zext i16 %load to i32
+  %icmp = icmp sgt i32 %sum.02, %ext
+  %min = select i1 %icmp, i32 %sum.02, i32 %ext
----------------
dmgreen wrote:
> Are we sure that smin/smax work correctly with this? We go from cutting off the top bits (potentially turning a signed number into an unsigned number) to not doing that.  Or maybe going from a signed min/max to a unsigned one, from the look of this test? I may be wrong, but I think something like this transform needs to be valid for the op: https://alive2.llvm.org/ce/z/5XJ4ZU.
> 
> umin/umax may still be OK, if the initial value from the phi is within range. But it doesn't reduce the vector width.
I think the smax/smin tests are working correctly, as we are still performing an `and` on the phi before the load & compare instructions to mask the top bits. In this test for smax I was previously using an inital value for the phi which was smaller than the mask, which meant the `and` was removed by instcombine. I've changed the initial value to make sure we can check for the `and` in this test which hopefully makes it a bit clearer.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105632/new/

https://reviews.llvm.org/D105632