[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa3cffc11509b: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) 2 (authored by hkmatsumoto, committed by spatel). Repository: rG LLVM

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto added a comment. @spatel Since I don't have commit access, can you land this patch? Please use "Hirochika Matsumoto as my identity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122077/new/ https://reviews.llvm.org/D122077

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel accepted this revision. spatel added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122077/new/ https://reviews.llvm.org/D122077

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto updated this revision to Diff 418869. hkmatsumoto added a comment. Create ctpop(X) > 1 instead of ctpop(X) >= 2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122077/new/ https://reviews.llvm.org/D122077 Files:

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:915 +/// Fold (icmp eq ctpop(X) 1) | (icmp eq X 0) into (icmp ult ctpop(X) 2) and +/// fold (icmp ne ctpop(X) 1) & (icmp ne X 0) into (icmp uge ctpop(X) 2). +static Value

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:915 +/// Fold (icmp eq ctpop(X) 1) | (icmp eq X 0) into (icmp ult ctpop(X) 2) and +/// fold (icmp ne ctpop(X) 1) & (icmp ne X 0) into (icmp uge ctpop(X) 2). +static Value

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added a comment. In D122077#3413565 , @xbolva00 wrote: > In D122077#3413550 , @joerg wrote: > >> Why is this fold preferable to `(X & (X-1)) == 0`? At least on all >> architectures without native

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto updated this revision to Diff 418832. hkmatsumoto added a comment. Sync with main branch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122077/new/ https://reviews.llvm.org/D122077 Files:

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

xbolva00 added a comment. In D122077#3413550 , @joerg wrote: > Why is this fold preferable to `(X & (X-1)) == 0`? At least on all > architectures without native population count, the binary-and based test is > preferable and it might even be better

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

joerg added a comment. Why is this fold preferable to `(X & (X-1)) == 0`? At least on all architectures without native population count, the binary-and based test is preferable and it might even be better with it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

RKSimon added a comment. rebase? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122077/new/ https://reviews.llvm.org/D122077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added a comment. Thank you for the making the test changes. I pushed the baseline tests on your behalf here: ebaa28e0750b Please rebase and update the patch here in Phabricator - it should only show changes in the

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto added a comment. In D122077#3405085 , @spatel wrote: > A few changes for tests suggested inline. > > There might be some generalization of ctpop analysis that we can make as a > follow-up patch. > For example, I was looking at a "wrong

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto added inline comments. Comment at: llvm/test/Transforms/InstCombine/ispow2.ll:540 - -define i1 @isnot_pow2_ctpop_wrong_pred1(i32 %x) { -; CHECK-LABEL: @isnot_pow2_ctpop_wrong_pred1( Renamed to is_pow2or0_ctpop. Comment at:

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto updated this revision to Diff 418405. hkmatsumoto added a comment. Reflect code reviews - Move all added tests to ispow2.ll from icmp-or.ll since now that they contain tests for "and" operand - Add tests to confirm extra uses don't affect the optimization - Add negative

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added a comment. A few changes for tests suggested inline. There might be some generalization of ctpop analysis that we can make as a follow-up patch. For example, I was looking at a "wrong predicate" combination and noticed that we miss possible optimizations like this:

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto marked 3 inline comments as done. hkmatsumoto added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:921 + if (IsAnd && + match(Cmp0, m_ICmp(Pred0, m_Intrinsic(m_Value(X)), + m_SpecificInt(1))) &&

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto updated this revision to Diff 417705. hkmatsumoto added a comment. Reflect code reviews - Factor out pattern matching code and exit early if it didn't match - Rename foldOrOfCtpop to foldIsPowerOf2OrZero - Rename 2 tests in ispow2.ll to is_pow2_or_zero(_logical) to inform that this

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

spatel added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:916 +/// fold (icmp ne ctpop(X) 1) & (icmp ne X 0) into (icmp uge ctpop(X) 2). +static Value *foldOrOfCtpop(ICmpInst *Cmp0, ICmpInst *Cmp1, bool IsAnd, +

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

craig.topper added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:921 + if (IsAnd && + match(Cmp0, m_ICmp(Pred0, m_Intrinsic(m_Value(X)), + m_SpecificInt(1))) && Since the m_ICmp matches are the

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto added inline comments. Comment at: llvm/test/Transforms/InstCombine/ispow2.ll:538 ; Negative test - wrong predicate (but this could reduce). xbolva00 wrote: > hkmatsumoto wrote: > > xbolva00 wrote: > > > Now reduced. > > Since this is almost the

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto updated this revision to Diff 416763. hkmatsumoto added a comment. Address code review, removing "but this could reduce" from comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122077/new/ https://reviews.llvm.org/D122077 Files:

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

xbolva00 added inline comments. Comment at: llvm/test/Transforms/InstCombine/ispow2.ll:538 ; Negative test - wrong predicate (but this could reduce). hkmatsumoto wrote: > xbolva00 wrote: > > Now reduced. > Since this is almost the first time contributing to

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto added inline comments. Comment at: llvm/test/Transforms/InstCombine/ispow2.ll:538 ; Negative test - wrong predicate (but this could reduce). xbolva00 wrote: > Now reduced. Since this is almost the first time contributing to LLVM, I'm not sure

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

hkmatsumoto updated this revision to Diff 416762. hkmatsumoto added a comment. Address code reviews - Fold (icmp ne ctpop(X) 1) & (icmp ne X 0) into (icmp ugt ctpop(X) 1) as well foldOrOfCtpop folds the aforementioned pattern into (icmp **uge** ctpop(X) 2) and that is later transformed into

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

craig.topper added inline comments. Comment at: llvm/test/Transforms/InstCombine/icmp-or.ll:382 + ret i1 %5 +} + RKSimon wrote: > What about if the ctpop has multi uses? The ctpop isn't being changed. Does it matter if it has more uses? What is interesting is

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

RKSimon added inline comments. Comment at: llvm/test/Transforms/InstCombine/icmp-or.ll:382 + ret i1 %5 +} + What about if the ctpop has multi uses? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122077/new/

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

craig.topper added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2613 + if (Value *V = foldOrOfCtpop(LHS, RHS, Builder)) +return V; Should we also handle the opposite version `ctpop(x) != 1 && x != 0`?

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

xbolva00 added inline comments. Comment at: llvm/test/Transforms/InstCombine/icmp-or.ll:371 + +define i1 @ctpop_or_eq_to_ctpop(i32 %0) { +; CHECK-LABEL: @ctpop_or_eq_to_ctpop( Please add a test with vector type. Comment at: