From 028b2aa56a627f6175dbe89ed37c2852a884281a Mon Sep 17 00:00:00 2001
From: Johannes Doerfert <jdoerfert@anl.gov>
Date: Tue, 20 Aug 2019 05:57:01 +0000
Subject: [PATCH] [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.

llvm-svn: 369327
---
 llvm/include/llvm/Transforms/IPO/Attributor.h | 9 ++++++---
 llvm/lib/Transforms/IPO/Attributor.cpp        | 6 ++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h
index 484dd0c31e25..7e03e54fde5d 100644
--- a/llvm/include/llvm/Transforms/IPO/Attributor.h
+++ b/llvm/include/llvm/Transforms/IPO/Attributor.h
@@ -890,10 +890,13 @@ struct IntegerState : public AbstractState {
   /// 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;
   }
diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 5ad9d4cb977a..ee3541b7c1db 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -526,6 +526,8 @@ struct AAReturnedFromReturnedValues : public AAType {
   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 : public AAType {
   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);
   }