[ValueLattice] Steal bits from Tag to track range extensions (NFC).

[fhahn]

Summary

Users of ValueLatticeElement currently have to ensure constant ranges
are not extended indefinitely. For example, in SCCP, mergeIn goes to
overdefined if a constantrange value is repeatedly merged with larger
constantranges. This is a simple form of widening.

In some cases, this leads to an unnecessary loss of information and
things can be improved by allowing a small number of extensions in the
hope that a fixed point is reached after a small number of steps.

To make better decisions about widening, it is helpful to keep track of
the number of range extensions. That state is tied directly to a
concrete ValueLatticeElement and some unused bits in the class can be
used. The current patch preserves the existing behavior by default:
CheckWiden defaults to false and if CheckWiden is true, a single change
to the range is allowed.

Follow-up patches will slightly increase the threshold for widening.

[efriedma]

LGTM

[fhahn]

Rebased

[nikic]

llvm/include/llvm/Analysis/ValueLattice.h

78–80

Why 6? I would expect either 3 here (because that's enough to represent the states) or 8 (an ISA friendly number).

[fhahn]

llvm/include/llvm/Analysis/ValueLattice.h

78–80

The main intention was to leave enough space for plenty of further extension while keeping plenty of other bits for other stuff. But it's probably worth to extend it to 8. I'll do that as a follow up commit.

llvm/include/llvm/Analysis/ValueLattice.h

enum ValueLatticeElementTy {
     ///  overdefined
     constantrange_including_undef,
 
     /// We can not precisely model the dynamic values this value might take.
     /// No transitions are allowed after reaching overdefined.
     overdefined,
   };
 
-  ValueLatticeElementTy Tag;
+  ValueLatticeElementTy Tag : 6;
+  /// Number of times a constant range has been extended with widening enabled.
+  unsigned NumRangeExtensions : 8;
 
   /// The union either stores a pointer to a constant or a constant range,
   /// associated to the lattice element. We have to ensure that Range is
   /// initialized or destroyed when changing state to or from constantrange.
   union {
     Constant *ConstVal;
     ConstantRange Range;
   };
ValueLatticeElement &operator=(const ValueLatticeElement &Other) {
 
     switch (Other.Tag) {
     case constantrange:
     case constantrange_including_undef:
       if (!isConstantRange())
         new (&Range) ConstantRange(Other.Range);
       else
         Range = Other.Range;
+      NumRangeExtensions = Other.NumRangeExtensions;
       break;
     case constant:
     case notconstant:
       ConstVal = Other.ConstVal;
       break;
     case overdefined:
     case unknown:
     case undef:
public:
   }
 
   /// Mark the object as constant 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 markConstantRange(ConstantRange NewR, bool MayIncludeUndef = false) {
+  bool markConstantRange(ConstantRange NewR, bool MayIncludeUndef = false,
+                         bool CheckWiden = false) {
     if (NewR.isFullSet())
       return markOverdefined();
 
     ValueLatticeElementTy OldTag = Tag;
     ValueLatticeElementTy NewTag =
         (isUndef() || isConstantRangeIncludingUndef() || MayIncludeUndef)
             ? constantrange_including_undef
             : constantrange;
     if (isConstantRange()) {
       if (NewR.isEmptySet())
         return markOverdefined();
 
       Tag = NewTag;
       if (getConstantRange() == NewR)
         return Tag != OldTag;
 
+      // Simple form of widening. If a range is extended multiple times, go to
+      // overdefined.
+      if (CheckWiden && ++NumRangeExtensions == 1)
+        return markOverdefined();
+
       assert(NewR.contains(getConstantRange()) &&
              "Existing range must be a subset of NewR");
       Range = std::move(NewR);
       return true;
     }
 
     assert(isUnknown() || isUndef());
     if (NewR.isEmptySet())
       return markOverdefined();
 
+    NumRangeExtensions = 0;
     Tag = NewTag;
     new (&Range) ConstantRange(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) {
+  bool mergeIn(const ValueLatticeElement &RHS, bool CheckWiden = false) {
     if (RHS.isUnknown() || isOverdefined())
       return false;
     if (RHS.isOverdefined()) {
       markOverdefined();
       return true;
     }
 
     if (isUndef()) {
       assert(!RHS.isUnknown());
       if (RHS.isUndef())
         return false;
       if (RHS.isConstant())
         return markConstant(RHS.getConstant(), /*MayIncludeUndef=*/true);
       if (RHS.isConstantRange())
         return markConstantRange(RHS.getConstantRange(true),
-                                 /*MayIncludeUndef=*/true);
+                                 /*MayIncludeUndef=*/true, CheckWiden);
       return markOverdefined();
     }
 
     if (isUnknown()) {
       assert(!RHS.isUnknown() && "Unknow RHS should be handled earlier");
       *this = RHS;
       return true;
     }
if (!RHS.isConstantRange()) {
       // and merge it with a constantrange.
       markOverdefined();
       return true;
     }
 
     ConstantRange NewR = getConstantRange().unionWith(RHS.getConstantRange());
     return markConstantRange(
         std::move(NewR),
-        /*MayIncludeUndef=*/RHS.isConstantRangeIncludingUndef());
+        /*MayIncludeUndef=*/RHS.isConstantRangeIncludingUndef(), CheckWiden);
   }
 
   // Compares this symbolic value with Other using Pred and returns either
   /// true, false or undef constants, or nullptr if the comparison cannot be
   /// evaluated.
   Constant *getCompare(CmpInst::Predicate Pred, Type *Ty,
                        const ValueLatticeElement &Other) const {
     if (isUnknownOrUndef() || Other.isUnknownOrUndef())
if (ConstantRange::makeSatisfyingICmpRegion(
             CmpInst::getInversePredicate(Pred), OtherCR)
             .contains(CR))
       return ConstantInt::getFalse(Ty);
 
     return nullptr;
   }
 };
 
-raw_ostream &operator<<(raw_ostream &OS, const ValueLatticeElement &Val);
+static_assert(sizeof(ValueLatticeElement) <= 40,
+              "size of ValueLatticeElement changed unexpectedly");
 
+raw_ostream &operator<<(raw_ostream &OS, const ValueLatticeElement &Val);
 } // end namespace llvm
 #endif

llvm/lib/Transforms/Scalar/SCCP.cpp

LLVM_DEBUG(dbgs() << "markOverdefined: ";
                else dbgs() << *V << '\n');
     // Only instructions go on the work list
     pushToWorkList(IV, V);
     return true;
   }
 
   bool mergeInValue(ValueLatticeElement &IV, Value *V,
                     ValueLatticeElement MergeWithV, bool Widen = true) {
-    // Do a simple form of widening, to avoid extending a range repeatedly in a
-    // loop. If IV is a constant range, it means we already set it once. If
-    // MergeWithV would extend IV, mark V as overdefined.
-    if (Widen && IV.isConstantRange() && MergeWithV.isConstantRange() &&
-        !IV.getConstantRange().contains(MergeWithV.getConstantRange())) {
-      markOverdefined(IV, V);
-      return true;
-    }
-    if (IV.mergeIn(MergeWithV)) {
+    if (IV.mergeIn(MergeWithV, Widen)) {
       pushToWorkList(IV, V);
       LLVM_DEBUG(dbgs() << "Merged " << MergeWithV << " into " << *V << " : "
                         << IV << "\n");
       return true;
     }
     return false;
   }