Skip to content

[IR] Add helper CmpPredicate::dropSameSign #134071

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 2, 2025
Merged

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Apr 2, 2025

Address review comment #133711 (comment)

@dtcxzyw dtcxzyw requested a review from artagnon April 2, 2025 11:26
@dtcxzyw dtcxzyw requested a review from nikic as a code owner April 2, 2025 11:26
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:ir llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Apr 2, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 2, 2025

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Address review comment #133711 (comment)


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

3 Files Affected:

  • (modified) llvm/include/llvm/IR/CmpPredicate.h (+4)
  • (modified) llvm/lib/Analysis/ScalarEvolution.cpp (+4-4)
  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+1-1)
diff --git a/llvm/include/llvm/IR/CmpPredicate.h b/llvm/include/llvm/IR/CmpPredicate.h
index 6921afbc58d25..62883a7365261 100644
--- a/llvm/include/llvm/IR/CmpPredicate.h
+++ b/llvm/include/llvm/IR/CmpPredicate.h
@@ -41,6 +41,10 @@ class CmpPredicate {
   /// Query samesign information, for optimizations.
   bool hasSameSign() const { return HasSameSign; }
 
+  /// Drops samesign information. This is used when the samesign information
+  /// should be dropped explicitly.
+  CmpInst::Predicate withoutSameSign() const { return Pred; }
+
   /// Compares two CmpPredicates taking samesign into account and returns the
   /// canonicalized CmpPredicate if they match. An alternative to operator==.
   ///
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 2bf738cd16d65..dc74f480f12bb 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -11622,7 +11622,7 @@ bool ScalarEvolution::isBasicBlockEntryGuardedByCond(const BasicBlock *BB,
   // to prove non-equality and non-strict comparison separately.
   CmpPredicate NonStrictPredicate = ICmpInst::getNonStrictCmpPredicate(Pred);
   const bool ProvingStrictComparison =
-      (Pred != static_cast<CmpInst::Predicate>(NonStrictPredicate));
+      Pred != NonStrictPredicate.withoutSameSign();
   bool ProvedNonStrictComparison = false;
   bool ProvedNonEquality = false;
 
@@ -11792,9 +11792,9 @@ bool ScalarEvolution::isImpliedCond(CmpPredicate Pred, const SCEV *LHS,
         const SCEV *TruncFoundLHS = getTruncateExpr(FoundLHS, NarrowType);
         const SCEV *TruncFoundRHS = getTruncateExpr(FoundRHS, NarrowType);
         // We cannot preserve samesign after truncation.
-        if (isImpliedCondBalancedTypes(
-                Pred, LHS, RHS, static_cast<ICmpInst::Predicate>(FoundPred),
-                TruncFoundLHS, TruncFoundRHS, CtxI))
+        if (isImpliedCondBalancedTypes(Pred, LHS, RHS,
+                                       FoundPred.withoutSameSign(),
+                                       TruncFoundLHS, TruncFoundRHS, CtxI))
           return true;
       }
     }
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index e75b4026d5424..663c8a38f5876 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -5641,7 +5641,7 @@ Instruction *InstCombinerImpl::foldICmpWithMinMax(Instruction &I,
   // execute comparisons. For example, `icmp samesign ult umax(X, -46), -32`
   // cannot be decomposed into `(icmp samesign ult X, -46) or (icmp samesign ult
   // -46, -32)`. `X` is allowed to be non-negative here.
-  Pred = static_cast<CmpInst::Predicate>(Pred);
+  Pred = Pred.withoutSameSign();
   auto CmpXZ = IsCondKnownTrue(simplifyICmpInst(Pred, X, Z, Q));
   auto CmpYZ = IsCondKnownTrue(simplifyICmpInst(Pred, Y, Z, Q));
   if (!CmpXZ.has_value() && !CmpYZ.has_value())

Copy link
Contributor

@artagnon artagnon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@dtcxzyw dtcxzyw changed the title [IR] Add helper CmpPredicate::withoutSameSign [IR] Add helper CmpPredicate::dropSameSign Apr 2, 2025
Copy link

github-actions bot commented Apr 2, 2025

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

@dtcxzyw dtcxzyw merged commit 65ed353 into llvm:main Apr 2, 2025
9 of 11 checks passed
@dtcxzyw dtcxzyw deleted the withoutSameSign branch April 2, 2025 14:25
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:ir llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants