[llvm] 6ba0695 - [ValueLattice] Add struct for merge options.

[Florian Hahn via llvm-commits]

Author: Florian Hahn
Date: 2020-04-19T09:03:16+01:00
New Revision: 6ba0695c600a41950336cc1abdc2c78c2a777d93

URL: https://github.com/llvm/llvm-project/commit/6ba0695c600a41950336cc1abdc2c78c2a777d93
DIFF: https://github.com/llvm/llvm-project/commit/6ba0695c600a41950336cc1abdc2c78c2a777d93.diff

LOG: [ValueLattice] Add struct for merge options.

This makes it easier to extend the merge options in the future and also
reduces the risk of accidentally setting a wrong option.

Reviewers: efriedma, nikic, reames, davide

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D78368

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/ValueLattice.h
    llvm/lib/Transforms/Scalar/SCCP.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/ValueLattice.h b/llvm/include/llvm/Analysis/ValueLattice.h
index d970fbfbb403..04230cddb901 100644
--- a/llvm/include/llvm/Analysis/ValueLattice.h
+++ b/llvm/include/llvm/Analysis/ValueLattice.h
@@ -88,6 +88,31 @@ class ValueLatticeElement {
   };
 
 public:
+  /// Struct to control some aspects related to merging constant ranges.
+  struct MergeOptions {
+    /// The merge value may include undef.
+    bool MayIncludeUndef;
+
+    /// Handle repeatedly extending a range by going to overdefined after a
+    /// number of steps.
+    bool CheckWiden;
+
+    MergeOptions() : MergeOptions(false, false) {}
+
+    MergeOptions(bool MayIncludeUndef, bool CheckWiden)
+        : MayIncludeUndef(MayIncludeUndef), CheckWiden(CheckWiden) {}
+
+    MergeOptions &setMayIncludeUndef(bool V = true) {
+      MayIncludeUndef = V;
+      return *this;
+    }
+
+    MergeOptions &setCheckWiden(bool V = true) {
+      CheckWiden = V;
+      return *this;
+    }
+  };
+
   // Const and Range are initialized on-demand.
   ValueLatticeElement() : Tag(unknown) {}
 
@@ -164,7 +189,8 @@ class ValueLatticeElement {
       return getOverdefined();
 
     ValueLatticeElement Res;
-    Res.markConstantRange(std::move(CR), MayIncludeUndef);
+    Res.markConstantRange(std::move(CR),
+                          MergeOptions().setMayIncludeUndef(MayIncludeUndef));
     return Res;
   }
   static ValueLatticeElement getOverdefined() {
@@ -248,7 +274,9 @@ class ValueLatticeElement {
     }
 
     if (ConstantInt *CI = dyn_cast<ConstantInt>(V))
-      return markConstantRange(ConstantRange(CI->getValue()), MayIncludeUndef);
+      return markConstantRange(
+          ConstantRange(CI->getValue()),
+          MergeOptions().setMayIncludeUndef(MayIncludeUndef));
 
     assert(isUnknown() || isUndef());
     Tag = constant;
@@ -282,14 +310,14 @@ class ValueLatticeElement {
   /// 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 CheckWiden = false) {
+  bool markConstantRange(ConstantRange NewR,
+                         MergeOptions Opts = MergeOptions()) {
     if (NewR.isFullSet())
       return markOverdefined();
 
     ValueLatticeElementTy OldTag = Tag;
     ValueLatticeElementTy NewTag =
-        (isUndef() || isConstantRangeIncludingUndef() || MayIncludeUndef)
+        (isUndef() || isConstantRangeIncludingUndef() || Opts.MayIncludeUndef)
             ? constantrange_including_undef
             : constantrange;
     if (isConstantRange()) {
@@ -302,7 +330,7 @@ class ValueLatticeElement {
 
       // Simple form of widening. If a range is extended multiple times, go to
       // overdefined.
-      if (CheckWiden && ++NumRangeExtensions == 1)
+      if (Opts.CheckWiden && ++NumRangeExtensions == 1)
         return markOverdefined();
 
       assert(NewR.contains(getConstantRange()) &&
@@ -323,7 +351,8 @@ class ValueLatticeElement {
 
   /// Updates this object to approximate both this object and RHS. Returns
   /// true if this object has been changed.
-  bool mergeIn(const ValueLatticeElement &RHS, bool CheckWiden = false) {
+  bool mergeIn(const ValueLatticeElement &RHS,
+               MergeOptions Opts = MergeOptions()) {
     if (RHS.isUnknown() || isOverdefined())
       return false;
     if (RHS.isOverdefined()) {
@@ -336,10 +365,10 @@ class ValueLatticeElement {
       if (RHS.isUndef())
         return false;
       if (RHS.isConstant())
-        return markConstant(RHS.getConstant(), /*MayIncludeUndef=*/true);
+        return markConstant(RHS.getConstant(), true);
       if (RHS.isConstantRange())
         return markConstantRange(RHS.getConstantRange(true),
-                                 /*MayIncludeUndef=*/true, CheckWiden);
+                                 Opts.setMayIncludeUndef());
       return markOverdefined();
     }
 
@@ -382,7 +411,7 @@ class ValueLatticeElement {
     ConstantRange NewR = getConstantRange().unionWith(RHS.getConstantRange());
     return markConstantRange(
         std::move(NewR),
-        /*MayIncludeUndef=*/RHS.isConstantRangeIncludingUndef(), CheckWiden);
+        Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef()));
   }
 
   // Compares this symbolic value with Other using Pred and returns either

diff  --git a/llvm/lib/Transforms/Scalar/SCCP.cpp b/llvm/lib/Transforms/Scalar/SCCP.cpp
index 187f26f7ca52..18431d0b70ff 100644
--- a/llvm/lib/Transforms/Scalar/SCCP.cpp
+++ b/llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -399,9 +399,13 @@ class SCCPSolver : public InstVisitor<SCCPSolver> {
     return true;
   }
 
+  /// Merge \p MergeWithV into \p IV and push \p V to the worklist, if \p IV
+  /// changes.
   bool mergeInValue(ValueLatticeElement &IV, Value *V,
-                    ValueLatticeElement MergeWithV, bool Widen = true) {
-    if (IV.mergeIn(MergeWithV, Widen)) {
+                    ValueLatticeElement MergeWithV,
+                    ValueLatticeElement::MergeOptions Opts = {
+                        /*MayIncludeUndef=*/false, /*CheckWiden=*/true}) {
+    if (IV.mergeIn(MergeWithV, Opts)) {
       pushToWorkList(IV, V);
       LLVM_DEBUG(dbgs() << "Merged " << MergeWithV << " into " << *V << " : "
                         << IV << "\n");
@@ -411,10 +415,11 @@ class SCCPSolver : public InstVisitor<SCCPSolver> {
   }
 
   bool mergeInValue(Value *V, ValueLatticeElement MergeWithV,
-                    bool Widen = true) {
+                    ValueLatticeElement::MergeOptions Opts = {
+                        /*MayIncludeUndef=*/false, /*CheckWiden=*/true}) {
     assert(!V->getType()->isStructTy() &&
            "non-structs should use markConstant");
-    return mergeInValue(ValueState[V], V, MergeWithV, Widen);
+    return mergeInValue(ValueState[V], V, MergeWithV, Opts);
   }
 
   /// getValueState - Return the ValueLatticeElement object that corresponds to
@@ -1203,7 +1208,8 @@ void SCCPSolver::handleCallArguments(CallSite CS) {
           mergeInValue(getStructValueState(&*AI, i), &*AI, CallArg);
         }
       } else
-        mergeInValue(&*AI, getValueState(*CAI), false);
+        mergeInValue(&*AI, getValueState(*CAI),
+                     ValueLatticeElement::MergeOptions().setCheckWiden(false));
     }
   }
 }