[PATCH] D18066: Add some shortcuts in Value Propagation for alloca

[Philip Reames via llvm-commits]

reames added a comment.

Really interesting observations and great idea, but the patch is not structured well.  In both cases, we're really talking about generic properties of LVI and we should structure it that way.  That should help all callers of LVI, not just these two.


================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:180
@@ -179,1 +179,3 @@
   if (isa<Constant>(Pointer)) return false;
+  // For pointer defined by AllocaInst, it cannot be a constant.
+  if (dyn_cast<AllocaInst>(Pointer))
----------------
This should be sunk inside LVI.  Specifically, at the top of LazyValueInfo::getConstant.

I'm also not entirely sure this is accurate.  I could construct a branch of the form:
if (%alloca == %some_constant_pointer) {
} else { }

Inside the if clause, I could find a constant value for the alloca.  It's just that we happen to know that the branch can't be true and thus that we'd only be able to simplify in dead code.

At minimum, we should have a better description here.  This is effectively a profitability heuristic more than anything else.

================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:324
@@ +323,3 @@
+        !isa<Constant>(V) &&
+        (dyn_cast<AllocaInst>(V) ||
+         LVI->getPredicateAt(ICmpInst::ICMP_EQ, V,
----------------
davidxl wrote:
> Have a helper function like:
> 
> bool isValueNonNull(V) -- which does simple checks.
> 
> So the combined check becomes:
> 
> if (mayHaveNullValue(...) && (isValueNonNull(V) || LVI ....)) {
> }
> 
> Note that isValueNonNull can also handle simple cases with gep:
> 
> foo() {
>    int a[10];
>     ...
>     bar(&a[5]);
>   ...
> }
> we know &a[5] can not be null.
> 
> 
This is the wrong place.  Rather than special casing this particular caller, we should really sink this down into LVI.  We could do this in one of two ways:
1) Use the non-context sensative form of isKnownNonNull in getPreciateAt when answering a non-null query.
2) Recognize the fact that the only fact we can infer about a pointer typed value is a constant or non-constant result.  The only non-constant we can get - I think - is not-null.  We can probably short circuit the search in this case.


Repository:
  rL LLVM

http://reviews.llvm.org/D18066