[llvm] r369327 - [Attributor] Fix the "clamp" operator

[Johannes Doerfert via llvm-commits]

Author: jdoerfert
Date: Mon Aug 19 22:57:01 2019
New Revision: 369327

URL: http://llvm.org/viewvc/llvm-project?rev=369327&view=rev
Log:
[Attributor] Fix the "clamp" operator

The clamp operator should not take the known of the given state as the
known is potentially based on assumed information. This also adds TODOs
to guide improvements.

Modified:
    llvm/trunk/include/llvm/Transforms/IPO/Attributor.h
    llvm/trunk/lib/Transforms/IPO/Attributor.cpp

Modified: llvm/trunk/include/llvm/Transforms/IPO/Attributor.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/IPO/Attributor.h?rev=369327&r1=369326&r2=369327&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/IPO/Attributor.h (original)
+++ llvm/trunk/include/llvm/Transforms/IPO/Attributor.h Mon Aug 19 22:57:01 2019
@@ -890,10 +890,13 @@ struct IntegerState : public AbstractSta
   /// Inequality for IntegerState.
   bool operator!=(const IntegerState &R) const { return !(*this == R); }
 
-  /// "Clamp" this state with \p R. The result is the maximum of the known
-  /// information but the minimum of the assumed.
+  /// "Clamp" this state with \p R. The result is the minimum of the assumed
+  /// information but not less than what was known before.
+  ///
+  /// TODO: Consider replacing the operator with a call or using it only when
+  ///       we can also take the maximum of the known information, thus when
+  ///       \p R is not dependent on additional assumed state.
   IntegerState operator^=(const IntegerState &R) {
-    takeKnownMaximum(R.Known);
     takeAssumedMinimum(R.Assumed);
     return *this;
   }

Modified: llvm/trunk/lib/Transforms/IPO/Attributor.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Attributor.cpp?rev=369327&r1=369326&r2=369327&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/Attributor.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/Attributor.cpp Mon Aug 19 22:57:01 2019
@@ -526,6 +526,8 @@ struct AAReturnedFromReturnedValues : pu
   ChangeStatus updateImpl(Attributor &A) override {
     StateType S;
     clampReturnedValueStates<AAType, StateType>(A, *this, S);
+    // TODO: If we know we visited all returned values, thus no are assumed
+    // dead, we can take the known information from the state T.
     return clampStateAndIndicateChange<StateType>(this->getState(), S);
   }
 };
@@ -585,6 +587,8 @@ struct AAArgumentFromCallSiteArguments :
   ChangeStatus updateImpl(Attributor &A) override {
     StateType S;
     clampCallSiteArgumentStates<AAType, StateType>(A, *this, S);
+    // TODO: If we know we visited all incoming values, thus no are assumed
+    // dead, we can take the known information from the state T.
     return clampStateAndIndicateChange<StateType>(this->getState(), S);
   }
 };
@@ -2265,6 +2269,8 @@ struct AAAlignFloating : AAAlignImpl {
                                                    VisitValueCB))
       indicatePessimisticFixpoint();
 
+    // TODO: If we know we visited all incoming values, thus no are assumed
+    // dead, we can take the known information from the state T.
     return clampStateAndIndicateChange(getState(), T);
   }