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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Other than the flags question, i think this is ok.



================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1181
+  }
+  return BinopElts();
+}
----------------
`return {};` doesn't work?
Elsewhere too, i don't think you need to specify `BinopElts`.


================
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]]
----------------
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.


https://reviews.llvm.org/D48662