Skip to content

Conversation

@wermos
Copy link
Contributor

@wermos wermos commented Dec 10, 2025

Both decomposeBitTestICmp and decomposeBitTest have a parameter called lookThroughTrunc. This was spelled in full (i.e. lookThroughTrunc) in the header (see here and here).

However, in the implementation, it's written as

bool LookThruTrunc, bool AllowNonZeroC,

and

I opted to convert all instances of lookThruTrunc into lookThroughTrunc to reduce surprise while reading the code and for conformity.


The other change in this PR is the renaming of the wrapper around decomposeBitTest(). Even though it was a wrapper around CmpInstAnalysis.h's decomposeBitTest, the function was called decomposeBitTestICmp. This is quite confusing because such a function also exists in CmpInstAnalysis.h, but it is not the one actually being used in InstCombineAndOrXor.cpp.

@wermos wermos requested a review from nikic as a code owner December 10, 2025 16:50
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Dec 10, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 10, 2025

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: Tirthankar Mazumder (wermos)

Changes

Both decomposeBitTestICmp and decomposeBitTest have a parameter called lookThroughTrunc. This was spelled in full (i.e. lookThroughTrunc) in the header (see here and here).

However, in the implementation, it's written as

bool LookThruTrunc, bool AllowNonZeroC,

and

I opted to convert all instances of lookThruTrunc into lookThroughTrunc to reduce surprise while reading the code and for conformity.


The other change in this PR is the renaming of the wrapper around decomposeBitTest(). Even though it was a wrapper around CmpInstAnalysis.h's decomposeBitTest, the function was called decomposeBitTestICmp. This is quite confusing because such a function also exists in CmpInstAnalysis.h, but it is not the one actually being used in InstCombineAndOrXor.cpp.


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

3 Files Affected:

  • (modified) llvm/lib/Analysis/CmpInstAnalysis.cpp (+4-4)
  • (modified) llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (+5-5)
  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+1-1)
diff --git a/llvm/lib/Analysis/CmpInstAnalysis.cpp b/llvm/lib/Analysis/CmpInstAnalysis.cpp
index a1a79e5685f80..a6d0d3ff4fcd4 100644
--- a/llvm/lib/Analysis/CmpInstAnalysis.cpp
+++ b/llvm/lib/Analysis/CmpInstAnalysis.cpp
@@ -75,7 +75,7 @@ Constant *llvm::getPredForFCmpCode(unsigned Code, Type *OpTy,
 
 std::optional<DecomposedBitTest>
 llvm::decomposeBitTestICmp(Value *LHS, Value *RHS, CmpInst::Predicate Pred,
-                           bool LookThruTrunc, bool AllowNonZeroC,
+                           bool LookThroughTrunc, bool AllowNonZeroC,
                            bool DecomposeAnd) {
   using namespace PatternMatch;
 
@@ -173,7 +173,7 @@ llvm::decomposeBitTestICmp(Value *LHS, Value *RHS, CmpInst::Predicate Pred,
     Result.Pred = ICmpInst::getInversePredicate(Result.Pred);
 
   Value *X;
-  if (LookThruTrunc && match(LHS, m_Trunc(m_Value(X)))) {
+  if (LookThroughTrunc && match(LHS, m_Trunc(m_Value(X)))) {
     Result.X = X;
     Result.Mask = Result.Mask.zext(X->getType()->getScalarSizeInBits());
     Result.C = Result.C.zext(X->getType()->getScalarSizeInBits());
@@ -185,7 +185,7 @@ llvm::decomposeBitTestICmp(Value *LHS, Value *RHS, CmpInst::Predicate Pred,
 }
 
 std::optional<DecomposedBitTest> llvm::decomposeBitTest(Value *Cond,
-                                                        bool LookThruTrunc,
+                                                        bool LookThroughTrunc,
                                                         bool AllowNonZeroC,
                                                         bool DecomposeAnd) {
   using namespace PatternMatch;
@@ -194,7 +194,7 @@ std::optional<DecomposedBitTest> llvm::decomposeBitTest(Value *Cond,
     if (!ICmp->getOperand(0)->getType()->isIntOrIntVectorTy())
       return std::nullopt;
     return decomposeBitTestICmp(ICmp->getOperand(0), ICmp->getOperand(1),
-                                ICmp->getPredicate(), LookThruTrunc,
+                                ICmp->getPredicate(), LookThroughTrunc,
                                 AllowNonZeroC, DecomposeAnd);
   }
   Value *X;
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index ba5568b00441b..64df974589055 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -186,8 +186,8 @@ static unsigned conjugateICmpMask(unsigned Mask) {
   return NewMask;
 }
 
-// Adapts the external decomposeBitTestICmp for local use.
-static bool decomposeBitTestICmp(Value *Cond, CmpInst::Predicate &Pred,
+// Adapts the external decomposeBitTest for local use.
+static bool decomposeBitTest(Value *Cond, CmpInst::Predicate &Pred,
                                  Value *&X, Value *&Y, Value *&Z) {
   auto Res = llvm::decomposeBitTest(Cond, /*LookThroughTrunc=*/true,
                                     /*AllowNonZeroC=*/true);
@@ -220,7 +220,7 @@ getMaskedTypeForICmpPair(Value *&A, Value *&B, Value *&C, Value *&D, Value *&E,
 
   // Check whether the icmp can be decomposed into a bit test.
   Value *L1, *L11, *L12, *L2, *L21, *L22;
-  if (decomposeBitTestICmp(LHS, PredL, L11, L12, L2)) {
+  if (decomposeBitTest(LHS, PredL, L11, L12, L2)) {
     L21 = L22 = L1 = nullptr;
   } else {
     auto *LHSCMP = dyn_cast<ICmpInst>(LHS);
@@ -253,7 +253,7 @@ getMaskedTypeForICmpPair(Value *&A, Value *&B, Value *&C, Value *&D, Value *&E,
     return std::nullopt;
 
   Value *R11, *R12, *R2;
-  if (decomposeBitTestICmp(RHS, PredR, R11, R12, R2)) {
+  if (decomposeBitTest(RHS, PredR, R11, R12, R2)) {
     if (R11 == L11 || R11 == L12 || R11 == L21 || R11 == L22) {
       A = R11;
       D = R12;
@@ -3890,7 +3890,7 @@ static std::optional<DecomposedBitMaskMul> matchBitmaskMul(Value *V) {
   // Decompose ((A & N) ? 0 : N * C) into BitMaskMul
   if (match(Op, m_Select(m_Value(Cond), m_APInt(EqZero), m_APInt(NeZero)))) {
     auto ICmpDecompose =
-        decomposeBitTest(Cond, /*LookThruTrunc=*/true,
+        decomposeBitTest(Cond, /*LookThroughTrunc=*/true,
                          /*AllowNonZeroC=*/false, /*DecomposeBitMask=*/true);
     if (!ICmpDecompose.has_value())
       return std::nullopt;
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index abf4381ebd794..1859dad4ec00b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -6290,7 +6290,7 @@ Instruction *InstCombinerImpl::foldICmpWithTrunc(ICmpInst &ICmp) {
 
   // This matches patterns corresponding to tests of the signbit as well as:
   // (trunc X) pred C2 --> (X & Mask) == C
-  if (auto Res = decomposeBitTestICmp(Op0, Op1, Pred, /*WithTrunc=*/true,
+  if (auto Res = decomposeBitTestICmp(Op0, Op1, Pred, /*LookThroughTrunc=*/true,
                                       /*AllowNonZeroC=*/true)) {
     Value *And = Builder.CreateAnd(Res->X, Res->Mask);
     Constant *C = ConstantInt::get(Res->X->getType(), Res->C);

@github-actions
Copy link

⚠️ 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 Developer Policy and LLVM Discourse for more information.

@github-actions
Copy link

github-actions bot commented Dec 10, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@wermos
Copy link
Contributor Author

wermos commented Dec 10, 2025

I am not sure what the problem with my email is, I have a publicly viewable email on my GitHub profile.

@andjo403
Copy link
Contributor

the problem with the mail is in the git commit the mail address is 63574588+wermos@users.noreply.github.com

@wermos
Copy link
Contributor Author

wermos commented Dec 11, 2025

Alright, I'll work on changing the commit email.

@wermos
Copy link
Contributor Author

wermos commented Dec 11, 2025

I've fixed the email issue. It should now show tmazumder.github@gmail.com as my email.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikic nikic enabled auto-merge (squash) December 11, 2025 07:18
@nikic nikic merged commit 2ce17ba into llvm:main Dec 11, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants