[PATCH] D86018: [Attributor] fix AAValueConstantRange and IntegerRangeState implementation

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

This patch addresses three unrelated things and we need to split it, assuming all points need to be addressed.



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:1956
     Known = Known.unionWith(R);
-    Assumed = Assumed.unionWith(Known);
   }
----------------
This looks dangerous as we might end up with a known range that is not subsumed in the assumed range.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:6964
+      auto &OpAA =
+          A.getAAFor<AAValueConstantRange>(*this, IRPosition::value(OpV));
+      intersectKnown(
----------------
okura wrote:
> uenoku wrote:
> > I'm not sure this propagation is necessary in `initialize` but at least, we need `TrackDpencdence = false` here. 
> For example, we can get [0,2) as a known range for `%a` instead of [0,inf) in the following case. And this propagation is needed for a value simplification in the case of `@potential_test3` in `potential.ll`.
> ```
> %a = zext i1 %bool to i32
> ```
> 
> > but at least, we need TrackDpencdence = false here.
> I'll fix that.
Why wouldn't that information be propagated during the update? Could you explain where it is lost?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86018/new/

https://reviews.llvm.org/D86018