[PATCH] D48662: [InstCombine] reverse canonicalization of binops to allow more shuffle folding

[Sanjay Patel via Phabricator via llvm-commits]

spatel planned changes to this revision.
spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1181
+  }
+  return BinopElts();
+}
----------------
lebedev.ri wrote:
> `return {};` doesn't work?
> Elsewhere too, i don't think you need to specify `BinopElts`.
That works. I wasn't sure if there's some LLVM pref/reason to do it this way (example: return SDValue() in codegen).


================
Comment at: test/Transforms/InstCombine/shuffle_select.ll:600
 ; CHECK-NEXT:    [[V0:%.*]] = lshr <4 x i8> [[V:%.*]], <i8 3, i8 3, i8 3, i8 3>
-; CHECK-NEXT:    [[T1:%.*]] = or <4 x i8> [[V0]], <i8 undef, i8 undef, i8 -64, i8 -64>
-; CHECK-NEXT:    [[T2:%.*]] = add nuw nsw <4 x i8> [[V0]], <i8 1, i8 2, i8 undef, i8 undef>
-; CHECK-NEXT:    [[T3:%.*]] = shufflevector <4 x i8> [[T1]], <4 x i8> [[T2]], <4 x i32> <i32 4, i32 5, i32 2, i32 3>
+; CHECK-NEXT:    [[T3:%.*]] = add nsw <4 x i8> [[V0]], <i8 1, i8 2, i8 -64, i8 -64>
 ; CHECK-NEXT:    ret <4 x i8> [[T3]]
----------------
lebedev.ri wrote:
> Too bad alive does not work with vectors :/
> I'm not sure i understand why we drop `nuw` but keep `nsw`.
> If anything, i'd guess the `or` is implicit `nuw`,
> so i would be less suprized if we dropped `nsw` instead.
This transform drops the flags since 'or' has no wrapping.
We add back the 'nsw' in visitAdd, but it doesn't find 'nuw' is possible (a shortcoming in computeOverflowForUnsignedAdd)...it should be both I think.


https://reviews.llvm.org/D48662