[PATCH] D48678: [InstCombine] enhance shuffle-of-binops to allow different variable ops (PR37806)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: test/Transforms/InstCombine/shuffle_select.ll:411-423
+; Div/rem need special handling if the shuffle has undef elements.
+
 define <4 x i32> @srem_2_vars(<4 x i32> %v0, <4 x i32> %v1) {
 ; CHECK-LABEL: @srem_2_vars(
-; CHECK-NEXT:    [[T1:%.*]] = srem <4 x i32> <i32 1, i32 2, i32 3, i32 4>, [[V0:%.*]]
-; CHECK-NEXT:    [[T2:%.*]] = srem <4 x i32> <i32 5, i32 6, i32 7, i32 8>, [[V1:%.*]]
-; CHECK-NEXT:    [[T3:%.*]] = shufflevector <4 x i32> [[T1]], <4 x i32> [[T2]], <4 x i32> <i32 0, i32 undef, i32 6, i32 3>
+; CHECK-NEXT:    [[TMP1:%.*]] = shufflevector <4 x i32> [[V0:%.*]], <4 x i32> [[V1:%.*]], <4 x i32> <i32 0, i32 undef, i32 6, i32 3>
+; CHECK-NEXT:    [[T3:%.*]] = srem <4 x i32> <i32 1, i32 1, i32 7, i32 4>, [[TMP1]]
 ; CHECK-NEXT:    ret <4 x i32> [[T3]]
----------------
lebedev.ri wrote:
> `%TMP1`'s second element is `undef`, since the mask is `undef`, no?
> So we are `srem`'ing by the vector with one known `undef`.
> That's ok?
Ah, good point. I don't think we currently have the analysis to cause trouble there, but we might in the future. 

So we need to replace the undef in the shuffle mask with the proper lane that selects from one of the source vectors. That might be forbidden because we'd be creating a more specific shuffle mask than already existed. Should we just give up on trying to fold div/rem in this case?


https://reviews.llvm.org/D48678