[PATCH] D79452: [VectorCombine] scalarize binop of inserted elements into vector constants

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 3 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/test/Transforms/VectorCombine/X86/insert-binop.ll:102
   %i0 = insertelement <2 x i64> <i64 42, i64 -42>, i64 %x, i64 1
   %i1 = insertelement <2 x i64> <i64 -7, i64 128>, i64 %y, i32 1
   %r = sdiv <2 x i64> %i0, %i1
----------------
RKSimon wrote:
> spatel wrote:
> > RKSimon wrote:
> > > Can we ever have a base vector with undef elements here? sdiv/udiv won't like that but I'm not sure if we can ever get in that state.
> > > ```
> > >   %i0 = insertelement <2 x i64> <i64 42, i64 undef>, i64 %x, i64 1
> > >   %i1 = insertelement <2 x i64> <i64 -7, i64 undef>, i64 %y, i32 1
> > > ```
> > Yes, it's ok to have undefs in the lane that we're inserting into, and ConstantFolding will deal with that. Demanded elements will eventually replace the unused constants that we see in this test with undefs.
> > 
> > We can't, however, have undefs in lanes we are not inserting into with div/rem - that would be immediate UB, so we would have simplified that before we reach here. But I can add tests with non-canonical IR to make sure there's nothing crazy happening here.
> Thanks, I think I was thinking that an undef value had the same effect as divide by zero in sdiv/udiv (causing the entire vector to become undef).
It does, and I was wrong about this getting folded sooner. I thought that analysis existed, but we don't have an InstSimplify that tries to recursively find undef/zero divisor elements and zap the entire value. We just do a simple check on vector constants. So the transform here could mask that potential simplification (although that seems like a rare possibility). I'll update with some test examples.


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

https://reviews.llvm.org/D79452