(ctpop(X) == N) || (X != 0) --> (X != 0) https://alive2.llvm.org/ce/z/hYL5C2 (for N = 10)
(ctpop(X) != N) && (X == 0) --> (X == 0) https://alive2.llvm.org/ce/z/oLkMvI (for N = 15)
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Proof (This is first time writing proof in English, so sorry for any inconveniences)
Suppose X: i32
- For 0 < N <= 32
- ctpop(X) == N --> X > 0.
- Hence (ctpop(X) == N) || (X != 0) --> X > 0 || X != 0.
- Intuitively X > 0 || X != 0 --> X != 0.
- For 32 < N (This case is already optimized anyway)
- ctpop(X) == N is always false, because of the semantics of ctpop().
- Hence (ctpop(X) == N) || (X != 0) --> false || X != 0 --> X != 0.
If we are not creating a new value in a transformation (only returning an existing value or constant), we usually prefer to add that fold (and tests) to "InstSimplify".
Note that you can use "llvm.assume" with Alive2 to create a more general proof instead of using a specific constant:
https://alive2.llvm.org/ce/z/vcY7_s
- Move change and its test to InstSimplify
- Remove/unchange tests whose and/or are replaced with select
This is because InstSimlify/and-or-icmp-* tests do not include such tests, and as far as I tried on goldbot.org simplifyAndOrOfICmps* functions in InstructionSimplify.cpp does not acknowledge select operands to optimize.
llvm/lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
1811 | Normally in this kind of code, we name this "C" (for constant). | |
1814 | Use m_ZeroInt() instead of m_SpecificInt()? | |
1815 | It seems more natural if we write this as "!C->isZero()" | |
llvm/test/Transforms/InstCombine/ispow2.ll | ||
900 ↗ | (On Diff #419469) | We should add a new test for "wrong predicate" because this test and the later one are reduced before we reach the transform in InstCombine. |
llvm/test/Transforms/InstSimplify/and-or-icmp-ctpop.ll | ||
35 | Should this be or <2 x i1> %cmp, %notzero? |
I posted inline comments on this patch yesterday, but it did not appear to trigger an email.
Oops, sorry I actually got the email but was hidden under a bunch of others.
Let me quickly address some of your reviews, and address all of them tomorrow morning.
Ah - I actually see the mail too now, but it was also buried in my inbox. :)
Thanks for the updates. I think you can request commit access now if you have not already:
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access
If you get that, you can add the new tests as an "NFC" patch first. After that, this patch should be good to commit too.
I pre-committed tests but immediately reverted as I noticed I forgot to rerun update_test_checks.py to make them baseline tests.
Let me take some time to confirm that the revert commit (rGf65c78a0949023bb0f3051cdaea7758e48420978) made CI pass.
Normally in this kind of code, we name this "C" (for constant).