[PATCH] D127119: [SLP]Fix undef handling in gather function.

[Harald van Dijk via Phabricator via llvm-commits]

hvdijk added inline comments.


================
Comment at: llvm/test/Transforms/SLPVectorizer/X86/diamond_broadcast_extra_shuffle.ll:37
 ; CHECK-NEXT:    [[TMP0:%.*]] = insertelement <4 x i32> poison, i32 [[LD]], i32 0
-; CHECK-NEXT:    [[SHUFFLE:%.*]] = shufflevector <4 x i32> [[TMP0]], <4 x i32> poison, <4 x i32> zeroinitializer
-; CHECK-NEXT:    [[TMP1:%.*]] = mul <4 x i32> [[SHUFFLE]], [[SHUFFLE]]
+; CHECK-NEXT:    [[SHUFFLE:%.*]] = shufflevector <4 x i32> [[TMP0]], <4 x i32> poison, <4 x i32> <i32 0, i32 0, i32 0, i32 undef>
+; CHECK-NEXT:    [[SHUFFLE1:%.*]] = shufflevector <4 x i32> [[TMP0]], <4 x i32> poison, <4 x i32> zeroinitializer
----------------
nlopes wrote:
> nikic wrote:
> > hvdijk wrote:
> > > vporpo wrote:
> > > > Doesn't an undef here mean that lane 4 can potentially be poison? Wouldn't that be incorrect?
> > > A mask of undef means the result is undef, not poison, even if the input contains poison elements. See https://llvm.org/docs/LangRef.html#shufflevector-instruction (which is less clear than I would have liked, it used to explicitly say undef, it now says "undefined" to be more readable, but it's not obvious that it is still meant to refer to undef rather than poison)
> > This will be switched to return a poison element in the future, so it would be best not to rely on it. (cc @nlopes).
> Right, thanks!
> We need to switch that behavior to fix a bunch of optimizations.
D93818 linked for reference. Okay, are you saying for the purpose of SLPVectorizer, you would prefer that we already treat undef masks as selecting poison? That should always result in code that is already correct today, but it may not always be optimal under the current rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127119