Skip to content
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

[LVI][SCCP][CVP] Add basic ConstantFPRange support #111544

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Oct 8, 2024

Note: Some nan-simplification tests will be broken if we convert fp constant into a range as we did for constant integers.

@llvmbot
Copy link
Member

llvmbot commented Oct 8, 2024

@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-function-specialization

Author: Yingwei Zheng (dtcxzyw)

Changes

Note: Some nan-simplification tests will be broken if we convert fp constant into a range as we did for constant integers.


Patch is 23.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/111544.diff

6 Files Affected:

  • (modified) llvm/include/llvm/Analysis/ValueLattice.h (+195-20)
  • (modified) llvm/include/llvm/IR/Constant.h (+6)
  • (modified) llvm/lib/Analysis/LazyValueInfo.cpp (+22)
  • (modified) llvm/lib/Analysis/ValueLattice.cpp (+46-20)
  • (modified) llvm/lib/IR/Constants.cpp (+39)
  • (modified) llvm/lib/Transforms/Utils/SCCPSolver.cpp (+8-1)
diff --git a/llvm/include/llvm/Analysis/ValueLattice.h b/llvm/include/llvm/Analysis/ValueLattice.h
index 9357a15f7619f1..3c5cfa7390815b 100644
--- a/llvm/include/llvm/Analysis/ValueLattice.h
+++ b/llvm/include/llvm/Analysis/ValueLattice.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_ANALYSIS_VALUELATTICE_H
 #define LLVM_ANALYSIS_VALUELATTICE_H
 
+#include "llvm/IR/ConstantFPRange.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/Constants.h"
 
@@ -38,6 +39,7 @@ class ValueLatticeElement {
     /// Transition allowed to the following states:
     ///  constant
     ///  constantrange_including_undef
+    ///  constantfprange_including_undef
     ///  overdefined
     undef,
 
@@ -70,6 +72,21 @@ class ValueLatticeElement {
     ///  overdefined
     constantrange_including_undef,
 
+    /// The Value falls within this range. (Used only for floating point typed
+    /// values.)
+    /// Transition allowed to the following states:
+    ///  constantfprange (new range must be a superset of the existing range)
+    ///  constantfprange_including_undef
+    ///  overdefined
+    constantfprange,
+
+    /// This Value falls within this range, but also may be undef.
+    /// Merging it with other constant ranges results in
+    /// constantfprange_including_undef.
+    /// Transition allowed to the following states:
+    ///  overdefined
+    constantfprange_including_undef,
+
     /// We can not precisely model the dynamic values this value might take.
     /// No transitions are allowed after reaching overdefined.
     overdefined,
@@ -85,6 +102,7 @@ class ValueLatticeElement {
   union {
     Constant *ConstVal;
     ConstantRange Range;
+    ConstantFPRange FPRange;
   };
 
   /// Destroy contents of lattice value, without destructing the object.
@@ -100,6 +118,10 @@ class ValueLatticeElement {
     case constantrange:
       Range.~ConstantRange();
       break;
+    case constantfprange_including_undef:
+    case constantfprange:
+      FPRange.~ConstantFPRange();
+      break;
     };
   }
 
@@ -154,6 +176,11 @@ class ValueLatticeElement {
       new (&Range) ConstantRange(Other.Range);
       NumRangeExtensions = Other.NumRangeExtensions;
       break;
+    case constantfprange:
+    case constantfprange_including_undef:
+      new (&FPRange) ConstantFPRange(Other.FPRange);
+      NumRangeExtensions = Other.NumRangeExtensions;
+      break;
     case constant:
     case notconstant:
       ConstVal = Other.ConstVal;
@@ -173,6 +200,11 @@ class ValueLatticeElement {
       new (&Range) ConstantRange(std::move(Other.Range));
       NumRangeExtensions = Other.NumRangeExtensions;
       break;
+    case constantfprange:
+    case constantfprange_including_undef:
+      new (&FPRange) ConstantFPRange(Other.FPRange);
+      NumRangeExtensions = Other.NumRangeExtensions;
+      break;
     case constant:
     case notconstant:
       ConstVal = Other.ConstVal;
@@ -225,6 +257,23 @@ class ValueLatticeElement {
                           MergeOptions().setMayIncludeUndef(MayIncludeUndef));
     return Res;
   }
+  static ValueLatticeElement getFPRange(ConstantFPRange CR,
+                                        bool MayIncludeUndef = false) {
+    if (CR.isFullSet())
+      return getOverdefined();
+
+    if (CR.isEmptySet()) {
+      ValueLatticeElement Res;
+      if (MayIncludeUndef)
+        Res.markUndef();
+      return Res;
+    }
+
+    ValueLatticeElement Res;
+    Res.markConstantFPRange(std::move(CR),
+                            MergeOptions().setMayIncludeUndef(MayIncludeUndef));
+    return Res;
+  }
   static ValueLatticeElement getOverdefined() {
     ValueLatticeElement Res;
     Res.markOverdefined();
@@ -239,6 +288,9 @@ class ValueLatticeElement {
   bool isConstantRangeIncludingUndef() const {
     return Tag == constantrange_including_undef;
   }
+  bool isConstantFPRangeIncludingUndef() const {
+    return Tag == constantfprange_including_undef;
+  }
   /// Returns true if this value is a constant range. Use \p UndefAllowed to
   /// exclude non-singleton constant ranges that may also be undef. Note that
   /// this function also returns true if the range may include undef, but only
@@ -247,6 +299,16 @@ class ValueLatticeElement {
     return Tag == constantrange || (Tag == constantrange_including_undef &&
                                     (UndefAllowed || Range.isSingleElement()));
   }
+  /// Returns true if this value is a constant floating point range. Use \p
+  /// UndefAllowed to exclude non-singleton constant ranges that may also be
+  /// undef. Note that this function also returns true if the range may include
+  /// undef, but only contains a single element. In that case, it can be
+  /// replaced by a constant.
+  bool isConstantFPRange(bool UndefAllowed = true) const {
+    return Tag == constantfprange ||
+           (Tag == constantfprange_including_undef &&
+            (UndefAllowed || FPRange.isSingleElement()));
+  }
   bool isOverdefined() const { return Tag == overdefined; }
 
   Constant *getConstant() const {
@@ -269,6 +331,17 @@ class ValueLatticeElement {
     return Range;
   }
 
+  /// Returns the constant floating point range for this value. Use \p
+  /// UndefAllowed to exclude non-singleton constant ranges that may also be
+  /// undef. Note that this function also returns a range if the range may
+  /// include undef, but only contains a single element. In that case, it can be
+  /// replaced by a constant.
+  const ConstantFPRange &getConstantFPRange(bool UndefAllowed = true) const {
+    assert(isConstantFPRange(UndefAllowed) &&
+           "Cannot get the constant-fprange of a non-constant-fprange!");
+    return FPRange;
+  }
+
   std::optional<APInt> asConstantInteger() const {
     if (isConstant() && isa<ConstantInt>(getConstant())) {
       return cast<ConstantInt>(getConstant())->getValue();
@@ -278,6 +351,15 @@ class ValueLatticeElement {
     return std::nullopt;
   }
 
+  std::optional<APFloat> asConstantFP() const {
+    if (isConstant() && isa<ConstantFP>(getConstant())) {
+      return cast<ConstantFP>(getConstant())->getValue();
+    } else if (isConstantFPRange() && getConstantFPRange().isSingleElement()) {
+      return *getConstantFPRange().getSingleElement();
+    }
+    return std::nullopt;
+  }
+
   ConstantRange asConstantRange(unsigned BW, bool UndefAllowed = false) const {
     if (isConstantRange(UndefAllowed))
       return getConstantRange();
@@ -288,11 +370,28 @@ class ValueLatticeElement {
     return ConstantRange::getFull(BW);
   }
 
+  ConstantFPRange asConstantFPRange(const fltSemantics &Sem,
+                                    bool UndefAllowed = false) const {
+    if (isConstantFPRange(UndefAllowed))
+      return getConstantFPRange();
+    if (isConstant())
+      return getConstant()->toConstantFPRange();
+    if (isUnknown())
+      return ConstantFPRange::getEmpty(Sem);
+    return ConstantFPRange::getFull(Sem);
+  }
+
   ConstantRange asConstantRange(Type *Ty, bool UndefAllowed = false) const {
     assert(Ty->isIntOrIntVectorTy() && "Must be integer type");
     return asConstantRange(Ty->getScalarSizeInBits(), UndefAllowed);
   }
 
+  ConstantFPRange asConstantFPRange(Type *Ty, bool UndefAllowed = false) const {
+    assert(Ty->isFPOrFPVectorTy() && "Must be floating point type");
+    return asConstantFPRange(Ty->getScalarType()->getFltSemantics(),
+                             UndefAllowed);
+  }
+
   bool markOverdefined() {
     if (isOverdefined())
       return false;
@@ -394,6 +493,51 @@ class ValueLatticeElement {
     return true;
   }
 
+  /// Mark the object as constant floating point range with \p NewR. If the
+  /// object is already a constant range, nothing changes if the existing range
+  /// is equal to \p NewR and the tag. Otherwise \p NewR must be a superset of
+  /// the existing range or the object must be undef. The tag is set to
+  /// constant_range_including_undef if either the existing value or the new
+  /// range may include undef.
+  bool markConstantFPRange(ConstantFPRange NewR,
+                           MergeOptions Opts = MergeOptions()) {
+    assert(!NewR.isEmptySet() && "should only be called for non-empty sets");
+
+    if (NewR.isFullSet())
+      return markOverdefined();
+
+    ValueLatticeElementTy OldTag = Tag;
+    ValueLatticeElementTy NewTag =
+        (isUndef() || isConstantFPRangeIncludingUndef() || Opts.MayIncludeUndef)
+            ? constantfprange_including_undef
+            : constantfprange;
+    if (isConstantFPRange()) {
+      Tag = NewTag;
+      if (getConstantFPRange() == NewR)
+        return Tag != OldTag;
+
+      // Simple form of widening. If a range is extended multiple times, go to
+      // overdefined.
+      if (Opts.CheckWiden && ++NumRangeExtensions > Opts.MaxWidenSteps)
+        return markOverdefined();
+
+      assert(NewR.contains(getConstantFPRange()) &&
+             "Existing range must be a subset of NewR");
+      FPRange = std::move(NewR);
+      return true;
+    }
+
+    assert(isUnknown() || isUndef() || isConstant());
+    assert(
+        (!isConstant() || NewR.contains(getConstant()->toConstantFPRange())) &&
+        "Constant must be subset of new range");
+
+    NumRangeExtensions = 0;
+    Tag = NewTag;
+    new (&FPRange) ConstantFPRange(std::move(NewR));
+    return true;
+  }
+
   /// Updates this object to approximate both this object and RHS. Returns
   /// true if this object has been changed.
   bool mergeIn(const ValueLatticeElement &RHS,
@@ -414,6 +558,9 @@ class ValueLatticeElement {
       if (RHS.isConstantRange())
         return markConstantRange(RHS.getConstantRange(true),
                                  Opts.setMayIncludeUndef());
+      if (RHS.isConstantFPRange())
+        return markConstantFPRange(RHS.getConstantFPRange(true),
+                                   Opts.setMayIncludeUndef());
       return markOverdefined();
     }
 
@@ -428,15 +575,26 @@ class ValueLatticeElement {
         return false;
       if (RHS.isUndef())
         return false;
-      // If the constant is a vector of integers, try to treat it as a range.
-      if (getConstant()->getType()->isVectorTy() &&
-          getConstant()->getType()->getScalarType()->isIntegerTy()) {
-        ConstantRange L = getConstant()->toConstantRange();
-        ConstantRange NewR = L.unionWith(
-            RHS.asConstantRange(L.getBitWidth(), /*UndefAllowed=*/true));
-        return markConstantRange(
-            std::move(NewR),
-            Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef()));
+      // If the constant is a vector of integers/floating point values, try to
+      // treat it as a range.
+      if (getConstant()->getType()->isVectorTy()) {
+        Type *ScalarTy = getConstant()->getType()->getScalarType();
+        if (ScalarTy->isIntegerTy()) {
+          ConstantRange L = getConstant()->toConstantRange();
+          ConstantRange NewR = L.unionWith(
+              RHS.asConstantRange(L.getBitWidth(), /*UndefAllowed=*/true));
+          return markConstantRange(
+              std::move(NewR),
+              Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef()));
+        }
+        if (ScalarTy->isFloatingPointTy()) {
+          ConstantFPRange L = getConstant()->toConstantFPRange();
+          ConstantFPRange NewR = L.unionWith(
+              RHS.asConstantFPRange(L.getSemantics(), /*UndefAllowed=*/true));
+          return markConstantFPRange(
+              std::move(NewR),
+              Opts.setMayIncludeUndef(RHS.isConstantFPRangeIncludingUndef()));
+        }
       }
       markOverdefined();
       return true;
@@ -450,18 +608,35 @@ class ValueLatticeElement {
     }
 
     auto OldTag = Tag;
-    assert(isConstantRange() && "New ValueLattice type?");
-    if (RHS.isUndef()) {
-      Tag = constantrange_including_undef;
-      return OldTag != Tag;
+    if (isConstantRange()) {
+      if (RHS.isUndef()) {
+        Tag = constantrange_including_undef;
+        return OldTag != Tag;
+      }
+
+      const ConstantRange &L = getConstantRange();
+      ConstantRange NewR = L.unionWith(
+          RHS.asConstantRange(L.getBitWidth(), /*UndefAllowed=*/true));
+      return markConstantRange(
+          std::move(NewR),
+          Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef()));
     }
 
-    const ConstantRange &L = getConstantRange();
-    ConstantRange NewR = L.unionWith(
-        RHS.asConstantRange(L.getBitWidth(), /*UndefAllowed=*/true));
-    return markConstantRange(
-        std::move(NewR),
-        Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef()));
+    if (isConstantFPRange()) {
+      if (RHS.isUndef()) {
+        Tag = constantfprange_including_undef;
+        return OldTag != Tag;
+      }
+
+      const ConstantFPRange &L = getConstantFPRange();
+      ConstantFPRange NewR = L.unionWith(
+          RHS.asConstantFPRange(L.getSemantics(), /*UndefAllowed=*/true));
+      return markConstantFPRange(
+          std::move(NewR),
+          Opts.setMayIncludeUndef(RHS.isConstantFPRangeIncludingUndef()));
+    } else {
+      llvm_unreachable("New ValueLattice type?");
+    }
   }
 
   // Compares this symbolic value with Other using Pred and returns either
@@ -492,7 +667,7 @@ class ValueLatticeElement {
   void setNumRangeExtensions(unsigned N) { NumRangeExtensions = N; }
 };
 
-static_assert(sizeof(ValueLatticeElement) <= 40,
+static_assert(sizeof(ValueLatticeElement) <= 80,
               "size of ValueLatticeElement changed unexpectedly");
 
 raw_ostream &operator<<(raw_ostream &OS, const ValueLatticeElement &Val);
diff --git a/llvm/include/llvm/IR/Constant.h b/llvm/include/llvm/IR/Constant.h
index 0aefb5ecf6b7f2..d6bde764d36d77 100644
--- a/llvm/include/llvm/IR/Constant.h
+++ b/llvm/include/llvm/IR/Constant.h
@@ -20,6 +20,7 @@
 namespace llvm {
 
 class ConstantRange;
+class ConstantFPRange;
 class APInt;
 
 /// This is an important base class in LLVM. It provides the common facilities
@@ -159,6 +160,11 @@ class Constant : public User {
   /// range is the union over the element ranges. Poison elements are ignored.
   ConstantRange toConstantRange() const;
 
+  /// Convert constant to an approximate constant floating point range. For
+  /// vectors, the range is the union over the element ranges. Poison elements
+  /// are ignored.
+  ConstantFPRange toConstantFPRange() const;
+
   /// Called if some element of this constant is no longer valid.
   /// At this point only other constants may be on the use_list for this
   /// constant.  Any constants on our Use list must also be destroy'd.  The
diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
index 30dc4ae30dbfa5..73ff6825ba7066 100644
--- a/llvm/lib/Analysis/LazyValueInfo.cpp
+++ b/llvm/lib/Analysis/LazyValueInfo.cpp
@@ -77,6 +77,9 @@ static bool hasSingleValue(const ValueLatticeElement &Val) {
       Val.getConstantRange().isSingleElement())
     // Integer constants are single element ranges
     return true;
+  if (Val.isConstantFPRange() && Val.getConstantFPRange().isSingleElement())
+    // Floating point constants are single element ranges
+    return true;
   if (Val.isConstant())
     // Non integer constants
     return true;
@@ -1685,6 +1688,11 @@ Constant *LazyValueInfo::getConstant(Value *V, Instruction *CxtI) {
     if (const APInt *SingleVal = CR.getSingleElement())
       return ConstantInt::get(V->getType(), *SingleVal);
   }
+  if (Result.isConstantFPRange()) {
+    const ConstantFPRange &CR = Result.getConstantFPRange();
+    if (const APFloat *SingleVal = CR.getSingleElement())
+      return ConstantFP::get(V->getType(), *SingleVal);
+  }
   return nullptr;
 }
 
@@ -1720,6 +1728,11 @@ Constant *LazyValueInfo::getConstantOnEdge(Value *V, BasicBlock *FromBB,
     if (const APInt *SingleVal = CR.getSingleElement())
       return ConstantInt::get(V->getType(), *SingleVal);
   }
+  if (Result.isConstantFPRange()) {
+    const ConstantFPRange &CR = Result.getConstantFPRange();
+    if (const APFloat *SingleVal = CR.getSingleElement())
+      return ConstantFP::get(V->getType(), *SingleVal);
+  }
   return nullptr;
 }
 
@@ -1751,6 +1764,15 @@ static Constant *getPredicateResult(CmpInst::Predicate Pred, Constant *C,
       return ConstantInt::getFalse(ResTy);
     return nullptr;
   }
+  if (Val.isConstantFPRange()) {
+    const ConstantFPRange &CR = Val.getConstantFPRange();
+    ConstantFPRange RHS = C->toConstantFPRange();
+    if (CR.fcmp(Pred, RHS))
+      return ConstantInt::getTrue(ResTy);
+    if (CR.fcmp(CmpInst::getInversePredicate(Pred), RHS))
+      return ConstantInt::getFalse(ResTy);
+    return nullptr;
+  }
 
   if (Val.isNotConstant()) {
     // If this is an equality comparison, we can try to fold it knowing that
diff --git a/llvm/lib/Analysis/ValueLattice.cpp b/llvm/lib/Analysis/ValueLattice.cpp
index 03810f1c554e5d..ba4b947f923c1c 100644
--- a/llvm/lib/Analysis/ValueLattice.cpp
+++ b/llvm/lib/Analysis/ValueLattice.cpp
@@ -40,15 +40,21 @@ ValueLatticeElement::getCompare(CmpInst::Predicate Pred, Type *Ty,
 
   // Integer constants are represented as ConstantRanges with single
   // elements.
-  if (!isConstantRange() || !Other.isConstantRange())
-    return nullptr;
-
-  const auto &CR = getConstantRange();
-  const auto &OtherCR = Other.getConstantRange();
-  if (CR.icmp(Pred, OtherCR))
-    return ConstantInt::getTrue(Ty);
-  if (CR.icmp(CmpInst::getInversePredicate(Pred), OtherCR))
-    return ConstantInt::getFalse(Ty);
+  if (isConstantRange() && Other.isConstantRange()) {
+    const auto &CR = getConstantRange();
+    const auto &OtherCR = Other.getConstantRange();
+    if (CR.icmp(Pred, OtherCR))
+      return ConstantInt::getTrue(Ty);
+    if (CR.icmp(CmpInst::getInversePredicate(Pred), OtherCR))
+      return ConstantInt::getFalse(Ty);
+  } else if (isConstantFPRange() && Other.isConstantFPRange()) {
+    const auto &CR = getConstantFPRange();
+    const auto &OtherCR = Other.getConstantFPRange();
+    if (CR.fcmp(Pred, OtherCR))
+      return ConstantInt::getTrue(Ty);
+    if (CR.fcmp(CmpInst::getInversePredicate(Pred), OtherCR))
+      return ConstantInt::getFalse(Ty);
+  }
 
   return nullptr;
 }
@@ -57,6 +63,9 @@ static bool hasSingleValue(const ValueLatticeElement &Val) {
   if (Val.isConstantRange() && Val.getConstantRange().isSingleElement())
     // Integer constants are single element ranges
     return true;
+  if (Val.isConstantFPRange() && Val.getConstantFPRange().isSingleElement())
+    // Floating point constants are single element ranges
+    return true;
   return Val.isConstant();
 }
 
@@ -94,19 +103,30 @@ ValueLatticeElement::intersect(const ValueLatticeElement &Other) const {
     return Other;
 
   // Could be either constant range or not constant here.
-  if (!isConstantRange() || !Other.isConstantRange()) {
-    // TODO: Arbitrary choice, could be improved
-    return *this;
+  if (isConstantRange() && Other.isConstantRange()) {
+    // Intersect two constant ranges
+    ConstantRange Range =
+        getConstantRange().intersectWith(Other.getConstantRange());
+    // Note: An empty range is implicitly converted to unknown or undef
+    // depending on MayIncludeUndef internally.
+    return ValueLatticeElement::getRange(
+        std::move(Range), /*MayIncludeUndef=*/isConstantRangeIncludingUndef() ||
+                              Other.isConstantRangeIncludingUndef());
+  }
+  if (isConstantFPRange() && Other.isConstantFPRange()) {
+    // Intersect two constant ranges
+    ConstantFPRange Range =
+        getConstantFPRange().intersectWith(Other.getConstantFPRange());
+    // Note: An empty range is implicitly converted to unknown or undef
+    // depending on MayIncludeUndef internally.
+    return ValueLatticeElement::getFPRange(
+        std::move(Range),
+        /*MayIncludeUndef=*/isConstantFPRangeIncludingUndef() ||
+            Other.isConstantFPRangeIncludingUndef());
   }
 
-  // Intersect two constant ranges
-  ConstantRange Range =
-      getConstantRange().intersectWith(Other.getConstantRange());
-  // Note: An empty range is implicitly converted to unknown or undef depending
-  // on MayIncludeUndef internally.
-  return ValueLatticeElement::getRange(
-      std::move(Range), /*MayIncludeUndef=*/isConstantRangeIncludingUndef() ||
-                            Other.isConstantRangeIncludingUndef());
+  // TODO: Arbitrary choice, could be improved
+  return *this;
 }
 
 raw_ostream &operator<<(...
[truncated]

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.

Can you please check the memory usage impact? I believe this increases the size of the lattice values by another two words, and IIRC both IPSCCP and LVI can be memory usage sensitive.

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Oct 8, 2024

Can you please check the memory usage impact? I believe this increases the size of the lattice values by another two words, and IIRC both IPSCCP and LVI can be memory usage sensitive.

I just pushed a new branch perf/lvi-cfr.

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Oct 8, 2024

Can you please check the memory usage impact? I believe this increases the size of the lattice values by another two words, and IIRC both IPSCCP and LVI can be memory usage sensitive.

I just pushed a new branch perf/lvi-cfr.

Compile-time impact:

instructions: http://llvm-compile-time-tracker.com/compare.php?from=a3a253d3c7780977077dd46493917b1949c0166d&to=553fdd7a60c0fc4cf379f284bc226ea427489cbd&stat=instructions:u

memory usage: http://llvm-compile-time-tracker.com/compare.php?from=a3a253d3c7780977077dd46493917b1949c0166d&to=553fdd7a60c0fc4cf379f284bc226ea427489cbd&stat=max-rss

@goldsteinn
Copy link
Contributor

Just a thought, but I think this would end up with a LOT less code duplication if you used templates for the implementations of int/fp CR helpers.

@@ -492,7 +667,7 @@ class ValueLatticeElement {
void setNumRangeExtensions(unsigned N) { NumRangeExtensions = N; }
};

static_assert(sizeof(ValueLatticeElement) <= 40,
static_assert(sizeof(ValueLatticeElement) <= 80,
Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering why the size increase was this large, and apparently the answer is: APFloat is currently throwing 8 bytes out of the window because it inherits from APFloatBase in a situation where empty base class optimization does not apply, because the first member if a union that contains members that also inherit from APFloatBase.

dtcxzyw added a commit that referenced this pull request Oct 9, 2024
Since both APFloat and (Double)IEEEFloat inherit from APFloatBase, empty
base optimization is not performed by GCC/Clang (Minimal reproducer:
https://godbolt.org/z/dY8cM3Wre). This patch removes inheritance
relation between (Double)IEEEFloat and APFloatBase to make sure EBO is
performed on APFloat. After this patch, the size of `ConstantFPRange`
will be reduced from 72 to 56.

Address comment
#111544 (comment).
@@ -492,7 +667,7 @@ class ValueLatticeElement {
void setNumRangeExtensions(unsigned N) { NumRangeExtensions = N; }
};

static_assert(sizeof(ValueLatticeElement) <= 40,
static_assert(sizeof(ValueLatticeElement) <= 64,
Copy link
Member Author

Choose a reason for hiding this comment

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

It looks better after #111641. But I believe we can further reduce the size of APFloat from 24 to 16.

@nikic
Copy link
Contributor

nikic commented Oct 9, 2024

Compile-time impact:

instructions: http://llvm-compile-time-tracker.com/compare.php?from=a3a253d3c7780977077dd46493917b1949c0166d&to=553fdd7a60c0fc4cf379f284bc226ea427489cbd&stat=instructions:u

Where does the extra cost come from? I wouldn't have expected this to have such a large impact, as FP optimizations usually don't significantly affect compile-time, and this PR doesn't hook up many things yet.

@dtcxzyw dtcxzyw changed the title [LVI][SCCP] Add basic ConstantFPRange support [LVI][SCCP][CVP] Add basic ConstantFPRange support Oct 9, 2024
@@ -233,21 +233,22 @@ static Value *getValueOnEdge(LazyValueInfo *LVI, Value *Incoming,
// value can never be that constant. In that case replace the incoming
// value with the other value of the select. This often allows us to
// remove the select later.
if (!SI->getType()->isFPOrFPVectorTy()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe better to make it an isIntOrIntVectorTy. Does this handle the pointer case below?

Copy link
Member Author

Choose a reason for hiding this comment

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

It also handles the pointer case. See also Transforms/CorrelatedValuePropagation/basic.ll:


; "true" case for CorrelatedValuePropagation
define void @loop2(ptr %x, ptr %y) {
; CHECK-LABEL: define void @loop2
; CHECK-SAME: (ptr [[X:%.*]], ptr [[Y:%.*]]) {
; CHECK-NEXT:  entry:
; CHECK-NEXT:    br label [[LOOP:%.*]]
; CHECK:       loop:
; CHECK-NEXT:    [[PHI:%.*]] = phi ptr [ [[F:%.*]], [[LOOP]] ], [ [[X]], [[ENTRY:%.*]] ]
; CHECK-NEXT:    [[F]] = tail call ptr @f(ptr [[PHI]])
; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq ptr [[F]], [[Y]]
; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[CMP1]], ptr null, ptr [[F]]
; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq ptr [[SEL]], null
; CHECK-NEXT:    br i1 [[CMP2]], label [[RETURN:%.*]], label [[LOOP]]
; CHECK:       return:
; CHECK-NEXT:    ret void
;
entry:
  br label %loop

loop:
  %phi = phi ptr [ %sel, %loop ], [ %x, %entry ]
  %f = tail call ptr @f(ptr %phi)
  %cmp1 = icmp eq ptr %f, %y
  %sel = select i1 %cmp1, ptr null, ptr %f
  %cmp2 = icmp eq ptr %sel, null
  br i1 %cmp2, label %return, label %loop

return:
  ret void
}

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.

5 participants