[PATCH] D134142: [InstCombine] Handle integer extension in `select` patterns using the condition as value

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D134142#3802487 <https://reviews.llvm.org/D134142#3802487>, @zero9178 wrote:

> Address review comments: Change approach to handling it in InstCombine. I noticed that these "implication" patterns for the condition being used as either the true or false values already existed but have simply only been handled for `i1` selection type while not taking `zext` and `sext` into account. 
> I now made these also handle `zext` and `sext` as well, making it capable of handling any integer type.

Nice improvement! Do you want to add a couple of tests to model your motivating example (where both arms of the select are extended)? Please pre-commit the tests with baseline output, then update the patch here, so it shows test diffs.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2671-2672
+    auto *One = ConstantInt::get(SelType, 1);
+    auto *Zero = Constant::getNullValue(SelType);
+    auto *AllOnes = Constant::getAllOnesValue(SelType);
+
----------------
Nit: I'd make all of these ConstantInt::get***() for consistency since we know all of them are integer types.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2683-2685
+    // select a, b, a -> select a, b, false
+    // select a, b, zext(a) -> select a, b, false
+    // select a, b, sext(a) -> select a, b, false
----------------
Change the "false" to "0" to be more general in these comments (and similar for "true" -> "1" below).


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

https://reviews.llvm.org/D134142