[PATCH] D48987: [InstCombine] drop poison flags for shuffle transforms with undefs

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: test/Transforms/InstCombine/shuffle_select.ll:30-42
 ; If any element of the shuffle mask operand is undef, that element of the result is undef.
 ; The shuffle is eliminated in this transform, but we can replace a constant element with undef.
-; Preserve flags when possible.
+; Preserve flags when possible. It's not safe to propagate poison-generating flags with undef constants.
 
 define <4 x i32> @mul(<4 x i32> %v) {
 ; CHECK-LABEL: @mul(
+; CHECK-NEXT:    [[S:%.*]] = mul <4 x i32> [[V:%.*]], <i32 undef, i32 12, i32 1, i32 14>
----------------
>>! In D48987#1153586, @spatel wrote:
>>>! In D48987#1153565, @lebedev.ri wrote:
>> So my concerns weren't unfounded :/
>> Do we want to also do such kind of fixup in `VisitShl()` in instcombine, for every `shl` we visit?
> 
> Do you have an example of the problem you're thinking of? AFAIK, we won't hit this problem with scalars or general transforms because they don't modify constants like we're doing here. We do have a related foldShuffledBinop() function that I will review/fix if this patch looks right.

https://godbolt.org/g/y7hCki

Should instcombine be dropping these flags just because?
Or we only care about not creating more of these situations than we already had?


https://reviews.llvm.org/D48987