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

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/Transforms/Scalar/CorrelatedValuePropagation.cpp:324
@@ +323,3 @@
+        !isa<Constant>(V) &&
+        (dyn_cast<AllocaInst>(V) ||
+         LVI->getPredicateAt(ICmpInst::ICMP_EQ, V,
----------------
reames wrote:
> 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.
I agree with you wrt to the final picture -- but should this be done in stages (to avoid over fixing the problem at hand)? Basically have a local fix now, get it stablized/tested and then move on to a general fix?  (I am fine either way)


Repository:
  rL LLVM

http://reviews.llvm.org/D18066