[PATCH] D129726: [InstCombine] (ShiftValC >> Y) >s -1 --> Y != 0 with ShiftValC < 0

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:2242
 
+    // (ShiftValC >> Y) >s -1 --> Y != 0 with ShiftValC < 0
+    if (!IsAShr && Pred == CmpInst::ICMP_SGT && C.isAllOnes() &&
----------------
We should also handle the related pattern that is checking if the shifted value is negative:
https://alive2.llvm.org/ce/z/bWfp-s

Using `InstCombiner::isSignBitCheck()` can reduce the code needed to match these patterns. Look around this file for examples of usage.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-shr.ll:1195
+
+; Negtive test
+define i1 @lshr_neg_sgt_minus_2(i8 %x) {
----------------
typo: "Negative"

You may pre-commit tests in "NFC" patches. You can push that kind of patch without pre-commit review on Phabricator. It would be good to test at least 2 more code patterns:
1. Use a vector type with "splat" constants.
2. Add an extra use of the shift.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129726