Skip to content

[ValueTracking] Squash compile-time regression from 66badf2 #122700

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
Jan 14, 2025

Conversation

artagnon
Copy link
Contributor

@artagnon artagnon commented Jan 13, 2025

66badf2 (VT: teach a special-case optz about samesign) introduced a compile-time regression due to the use of CmpPredicate::getMatching, which is unnecessarily inefficient. Introduce CmpPredicate::getPreferredSignedPredicate, which alleviates the inefficiency problem and squashes the compile-time regression.

66badf2 (VT: teach a special-case optz about samesign) introduced a
compile-time regression due to the use of CmpPredicate::getMatching,
which is unnecessarily inefficient. Introduce
CmpPredicate::getSignedPredicate, which alleviates the inefficiency
problem and squashes the compile-time regression.
@llvmbot
Copy link
Member

llvmbot commented Jan 13, 2025

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-analysis

Author: Ramkumar Ramachandra (artagnon)

Changes

66badf2 (VT: teach a special-case optz about samesign) introduced a compile-time regression due to the use of CmpPredicate::getMatching, which is unnecessarily inefficient. Introduce CmpPredicate::getSignedPredicate, which alleviates the inefficiency problem and squashes the compile-time regression.


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

3 Files Affected:

  • (modified) llvm/include/llvm/IR/CmpPredicate.h (+6)
  • (modified) llvm/lib/Analysis/ValueTracking.cpp (+5-4)
  • (modified) llvm/lib/IR/Instructions.cpp (+4)
diff --git a/llvm/include/llvm/IR/CmpPredicate.h b/llvm/include/llvm/IR/CmpPredicate.h
index 9aa1449465f5ff..829339088e7c37 100644
--- a/llvm/include/llvm/IR/CmpPredicate.h
+++ b/llvm/include/llvm/IR/CmpPredicate.h
@@ -53,6 +53,12 @@ class CmpPredicate {
   static std::optional<CmpPredicate> getMatching(CmpPredicate A,
                                                  CmpPredicate B);
 
+  /// Attempts to return a signed CmpInst::Predicate from the CmpPredicate. If
+  /// the CmpPredicate has samesign, return the signedness-flipped predicate
+  /// (which would be signed), dropping samesign information. Otherwise, return
+  /// the predicate, dropping samesign information.
+  CmpInst::Predicate getSignedPredicate() const;
+
   /// An operator== on the underlying Predicate.
   bool operator==(CmpInst::Predicate P) const { return Pred == P; }
   bool operator!=(CmpInst::Predicate P) const { return Pred != P; }
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 0e50fc60ce7921..0fabe45628b01e 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -9495,8 +9495,9 @@ isImpliedCondICmps(const ICmpInst *LHS, CmpPredicate RPred, const Value *R0,
   // must be positive if X >= Y and no overflow".
   // Take SGT as an example:  L0:x > L1:y and C >= 0
   //                      ==> R0:(x -nsw y) < R1:(-C) is false
-  if ((CmpPredicate::getMatching(LPred, ICmpInst::ICMP_SGT) ||
-       CmpPredicate::getMatching(LPred, ICmpInst::ICMP_SGE)) &&
+  CmpInst::Predicate SignedLPred = LPred.getSignedPredicate();
+  if ((SignedLPred == ICmpInst::ICMP_SGT ||
+       SignedLPred == ICmpInst::ICMP_SGE) &&
       match(R0, m_NSWSub(m_Specific(L0), m_Specific(L1)))) {
     if (match(R1, m_NonPositive()) &&
         isImpliedCondMatchingOperands(LPred, RPred) == false)
@@ -9505,8 +9506,8 @@ isImpliedCondICmps(const ICmpInst *LHS, CmpPredicate RPred, const Value *R0,
 
   // Take SLT as an example:  L0:x < L1:y and C <= 0
   //                      ==> R0:(x -nsw y) < R1:(-C) is true
-  if ((CmpPredicate::getMatching(LPred, ICmpInst::ICMP_SLT) ||
-       CmpPredicate::getMatching(LPred, ICmpInst::ICMP_SLE)) &&
+  if ((SignedLPred == ICmpInst::ICMP_SLT ||
+       SignedLPred == ICmpInst::ICMP_SLE) &&
       match(R0, m_NSWSub(m_Specific(L0), m_Specific(L1)))) {
     if (match(R1, m_NonNegative()) &&
         isImpliedCondMatchingOperands(LPred, RPred) == true)
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index 49c148bb68a4d3..b78e045f9c9190 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -3939,6 +3939,10 @@ std::optional<CmpPredicate> CmpPredicate::getMatching(CmpPredicate A,
   return {};
 }
 
+CmpInst::Predicate CmpPredicate::getSignedPredicate() const {
+  return HasSameSign ? ICmpInst::getFlippedSignednessPredicate(Pred) : Pred;
+}
+
 CmpPredicate CmpPredicate::get(const CmpInst *Cmp) {
   if (auto *ICI = dyn_cast<ICmpInst>(Cmp))
     return ICI->getCmpPredicate();

@artagnon artagnon changed the title VT: squah compile-time regression from 66badf2 VT: squash compile-time regression from 66badf2 Jan 13, 2025
@dtcxzyw
Copy link
Member

dtcxzyw commented Jan 13, 2025

Average speedup: -0.15%

@nikic nikic changed the title VT: squash compile-time regression from 66badf2 [ValueTracking] Squash compile-time regression from 66badf2 Jan 13, 2025
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

@artagnon artagnon merged commit 0e7b754 into llvm:main Jan 14, 2025
8 checks passed
@artagnon artagnon deleted the ct-vt-samesign-special-case branch January 14, 2025 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants