[PATCH] D126617: [InstCombine] Optimize shl+lshr+and conversion pattern

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 8 11:06:29 PDT 2022


spatel added a comment.

In D126617#3565340 <https://reviews.llvm.org/D126617#3565340>, @bcl5980 wrote:

> 1. Use m_Power2 to match C1 <https://reviews.llvm.org/C1>
> 2. Remove condition Log2(C1 <https://reviews.llvm.org/C1>) < Log2(C3)+C2

Update the Alive2 proof in the patch description, so it matches the new code.
Will you add the pattern where the shift order is reversed in another patch? ( https://alive2.llvm.org/ce/z/fNdbfZ )
You can put a TODO comment with the code in this patch, so we know it is should be added for symmetry.



================
Comment at: llvm/test/Transforms/InstCombine/and.ll:1625
 
+; CTTZ(ShlC) < LShrC
 define i16 @shl_lshr_pow2_const_case1(i16 %x) {
----------------
Do we have a negative test with "cttz(ShlC) > LShrC"? If not, please add that. If the test is already here, then add a comment like that, so we know the purpose of the test.


================
Comment at: llvm/test/Transforms/InstCombine/and.ll:1639
+; LShrC < CTTZ(ShlC) < LShrC + CTTZ(AndC)
+define i16 @shl_lshr_pow2_const_case2(i16 %x) {
+; CHECK-LABEL: @shl_lshr_pow2_const_case2(
----------------
This test is already optimized with D127122 ? It's fine to add another test, but please pre-commit before this patch, so we know how this patch alone is changing the tests.


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

https://reviews.llvm.org/D126617



More information about the llvm-commits mailing list