[PATCH] D152658: [InstCombine] Change SimplifyDemandedVectorElts to use PoisonElts instead of UndefElts

[Manuel Brito via Phabricator via cfe-commits]

ManuelJBrito added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstCombineIntrinsic.cpp:3102-3105
   case Intrinsic::x86_sse4a_extrqi:
   case Intrinsic::x86_sse4a_insertq:
   case Intrinsic::x86_sse4a_insertqi:
+    PoisonElts.setHighBits(VWidth / 2);
----------------
nlopes wrote:
> this change doesn't look correct. The bits are undefined, doesn't seem like you can use poison here.
Thanks, I'll fix it. This will cause some regressions in the cases where only the upper half is demanded.
Eventually we could fold those cases to freeze(poison).


================
Comment at: llvm/test/Transforms/InstCombine/vec_shuffle.ll:1301
 ; CHECK-LABEL: @fmul_splat_constant(
-; CHECK-NEXT:    [[TMP1:%.*]] = fmul <2 x float> [[X:%.*]], <float 4.200000e+01, float poison>
-; CHECK-NEXT:    [[R:%.*]] = shufflevector <2 x float> [[TMP1]], <2 x float> poison, <2 x i32> zeroinitializer
+  ; CHECK-NEXT:    [[TMP1:%.*]] = shufflevector <2 x float> [[X:%.*]], <2 x float> undef, <2 x i32> zeroinitializer
+; CHECK-NEXT:    [[R:%.*]] = fmul <2 x float> [[TMP1]], <float 4.200000e+01, float 4.200000e+01>
----------------
nlopes wrote:
> Why does it regress here (goes from poison to undef)?
This regression is due to the change in foldVectorBinop in InstructionCombining.cpp
It seems that without this canonicalization, demanded elements doesn't trigger causing the regression.

I changed it to match poison instead of undef, because of the following:

shufflevector < 4 x i32 > %1, < 4 x i32 > undef , <1, 2, 6, 7>  --> shufflevector < 4 x i32 > %1, < 4 x i32 > undef , <1, 2, undef, undef>

Now we don't do this because it would insert poison.

Not simplifying the mask causes the following assert to fail:

```
assert(ShMask[I] < (int)NumElts && "Not expecting narrowing shuffle") 
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152658