[PATCH] D69686: [LVI][CVP] Use block value in getPredicateAt()

Tue Nov 5 02:53:24 PST 2019

nikic marked an inline comment as done.
nikic added a comment.

In D69686#1732629 <https://reviews.llvm.org/D69686#1732629>, @reames wrote:

> Can you give some context on the problem you're trying to solve?  This doesn't look quite right, but maybe with some context I can make a suggestion as to how to approach cleanly?

I want to determine the result of icmps based on range information within the same basic block (see the two tests for examples). LVI has all the necessary machinery to do that, but right now that getPredicateAt() method which is used to evaluate icmps in JT and CVP does not actually make use of it. getPrediateAt() is (mostly) weaker than manually evaluating the predicate based on the getConstantRange() result, and CVP actually does that for some of the "non-negative" optimizations.

See also D44252 <https://reviews.llvm.org/D44252>, where you suggested (I think, if I understood correctly) to do what I'm trying to do here.

================
Comment at: llvm/lib/Analysis/LazyValueInfo.cpp:1865
+
+  // The getValueInBlock() result is valid anywhere in the block for non-pointer
+  // types. For pointer types, it will additionally take into account pointer
----------------
reames wrote:
> Wait, no, please don't do this.  Please don't bake in assumption about the semantics of the function based on the type of the argument.  If this difference exists, we should fix/remove it.  
> 
> I've long thought we needed to have a getValueAtBegin(BB) variant.  Would that solve your use case?
I agree that this is pretty weird. I can take a look at fixing the behavior of pointers, which I believe are in the wrong here (the integer behavior is already heavily embedded in how CVP works).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69686