[PATCH] D127073: [SLP] Treat undef as any other constant

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added inline comments.


================
Comment at: llvm/test/Transforms/SLPVectorizer/X86/diamond_broadcast_extra_shuffle.ll:121
+; CHECK-NEXT:    [[TMP0:%.*]] = insertelement <4 x i32> <i32 poison, i32 undef, i32 poison, i32 poison>, i32 [[LD]], i32 0
+; CHECK-NEXT:    [[SHUFFLE:%.*]] = shufflevector <4 x i32> [[TMP0]], <4 x i32> poison, <4 x i32> <i32 0, i32 0, i32 0, i32 1>
+; CHECK-NEXT:    [[SHUFFLE1:%.*]] = shufflevector <4 x i32> [[TMP0]], <4 x i32> poison, <4 x i32> <i32 0, i32 0, i32 1, i32 0>
----------------
hvdijk wrote:
> nlopes wrote:
> > this is also a regression.
> > Before we were doing a broadcast of %ld and doing `mul %ld, %ld` in all 4 lanes, as the undefs were replaced with %ld.
> Replacing undef with %ld is only valid if we can prove %ld is not poison, and we have no checks for that. It might be valid here, but it is not valid in general, is it?
True in general. Here it's fine because %ld is already present in the expression tree.
Plus these days clang emits the noundef attribute, so it's worth checking for it.

FWIW, the good news is that there's no information loss, so we can have instcombine recover from some of these patterns. But I would suggest checking for `isGuaranteedNotToBePoison()` (cf ValueTracking).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127073