[PATCH] D130433: [InstCombine] Add fold for redundant sign bits count comparison

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

The pre-conditions don't seem right - need more tests and pre-commit the baseline tests once those are settled, so we don't have noise from the other tests.

Does the shift amount matter as long as it is non-zero?
https://alive2.llvm.org/ce/z/dnt2g4

There must be an inverted predicate (ugt) pattern that should be handled for consistency?



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:1674
+  const APInt *ShiftC;
+  if (!match(Xor, m_c_Xor(m_Value(X), m_AShr(m_Deferred(X), m_APInt(ShiftC)))))
+    return nullptr;
----------------
majnemer wrote:
> Might be simpler with `m_ConstantInt(Shift)` so you don't need to explicitly unwrap the APInt.
Using m_ConstantInt() is the opposite of recent pattern-matching changes. This should use m_APInt(), so the transform is consistent for vector types (with splat constants).

Please add a test with vector types that is equivalent or close to a scalar test, so we know the transform works with vectors too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130433