[InstCombine] Infer exact for lshr by cttz

[houngkoungting]

Fix #131444
This patch adds a pattern . It match to infer the 'exact' flag on an 'lshr' instruction when the shift amount is computed via a 'cttz' intrinsic on the same operand.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: 黃國庭 (houngkoungting)

Changes

Fix #131444
This patch adds a pattern . It match to infer the 'exact' flag on an 'lshr' instruction when the shift amount is computed via a 'cttz' intrinsic on the same operand.

---

Full diff: https://github.com/llvm/llvm-project/pull/136696.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp (+12)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
index 90cd279e8a457..c39b7dae434e0 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -994,6 +994,18 @@ static bool setShiftFlags(BinaryOperator &I, const SimplifyQuery &Q) {
       I.setIsExact();
       return true;
     }
+    //Fix #131444
+    if (auto *Cttz = dyn_cast<IntrinsicInst>(I.getOperand(1))) {
+      if (Cttz->getIntrinsicID() == Intrinsic::cttz &&
+          Cttz->getOperand(0) == I.getOperand(0)) {
+        if (auto *Const = dyn_cast<ConstantInt>(Cttz->getOperand(1))) {
+          if (Const->isOne()) {  
+            I.setIsExact();
+            return true;
+          }
+        }
+      }
+    }
   }
 
   // Compute what we know about shift count.

[tclin914]

Could you provide the testcase for this change

[nikic]

Note that an existing InstCombine test is failing as well, please update it.

[nikic]

Don't reference the issue, instead add a compact comment on what the transform does. Like // Infer exact on shift by cttz.

[nikic]

You can use the m_Intrinsic matcher.

[nikic]

Why do we need the second parameter to be one? Can you please also add an alive2 proof for the transform to the PR description?

[houngkoungting]

HI @nikic , I think it has to be this way, because if i8 src = 0 and we shift by 8 bits, it doesn't feel right.
From https://llvm.org/docs/LangRef.html#llvm-cttz-intrinsic

[github-actions]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp -- llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
index c39b7dae4..4faa719fa 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -994,12 +994,12 @@ static bool setShiftFlags(BinaryOperator &I, const SimplifyQuery &Q) {
       I.setIsExact();
       return true;
     }
-    //Fix #131444
+    // Fix #131444
     if (auto *Cttz = dyn_cast<IntrinsicInst>(I.getOperand(1))) {
       if (Cttz->getIntrinsicID() == Intrinsic::cttz &&
           Cttz->getOperand(0) == I.getOperand(0)) {
         if (auto *Const = dyn_cast<ConstantInt>(Cttz->getOperand(1))) {
-          if (Const->isOne()) {  
+          if (Const->isOne()) {
             I.setIsExact();
             return true;
           }

[dtcxzyw]

Please add some tests to demonstrate the change. See also https://llvm.org/docs/InstCombineContributorGuide.html#tests

[houngkoungting]

HI @nikic @tclin914 @dtcxzyw , I think I missed several steps.
Should I close this PR and open a new one with everything fixed? Thanks!

[dtcxzyw]

Should I close this PR and open a new one with everything fixed? Thanks!

It is okay to fix them in this PR.