[PATCH] [CaptureTracking] Avoid long compilation time on large basic blocks

[Bruno Cardoso Lopes]

On Tue, May 5, 2015 at 7:10 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> As an incremental solution, this looks like a good idea.
>
> However, overall, this whole thing is, IMHO, the wrong solution.
> Trying to compute a stateful problem with a stateless API is always
> going to be slow :)
> Caching does not really fix this.

Yep

> The right way is to compute capturing as a stateful constraint/other
problem,
> once,  and if we discover we invalidate info too much, compute it again.
>
> This is like calling it "lazy value info" and then computing it on
> every variable, 5 times :)
>
> At the point at which you compute it for everything, it's no longer
> lazy, and it doesn't make a whole lot of sense to try to treat it as a
> lazy problem.

True! Take a look at some comments:

lib/Analysis/AliasAnalysis.cpp:
---
// *FIXME:* this is really just shoring-up a deficiency in alias analysis.
// BasicAA isn't willing to spend linear time determining whether an alloca
// was captured before or after this particular call, while we are. However,
// with a smarter AA in place, this test is just wasting compile time.
AliasAnalysis::ModRefResult
AliasAnalysis::*callCapturesBefore*(const Instruction *I,
                                  const AliasAnalysis::Location &MemLoc,
                                  DominatorTree *DT) {
  if (!DT)
    return AliasAnalysis::ModRef;

  const Value *Object = GetUnderlyingObject(MemLoc.Ptr, *DL);
  if (!isIdentifiedObject(Object) || isa<GlobalValue>(Object) ||
      isa<Constant>(Object))
    return AliasAnalysis::ModRef;

  ImmutableCallSite CS(I);
  if (!CS.getInstruction() || CS.getInstruction() == Object)
    return AliasAnalysis::ModRef;

  if (llvm::*PointerMayBeCapturedBefore*(Object, /* ReturnCaptures */ true,
                                       /* StoreCaptures */ true, I, DT,
                                       /* include Object */ true))
...
---

lib/Analysis/CaptureTracking.cpp
---
bool llvm::*PointerMayBeCapturedBefore*(const Value *V, bool ReturnCaptures,
                                      bool StoreCaptures, const Instruction
*I,
                                      DominatorTree *DT, bool IncludeI) {
  assert(!isa<GlobalValue>(V) &&
         "It doesn't make sense to ask whether a global is captured.");

  if (!DT)
    return PointerMayBeCaptured(V, ReturnCaptures, StoreCaptures);

  // *TODO:* See comment in PointerMayBeCaptured regarding what could be
done
  // with StoreCaptures.

  CapturesBefore CB(ReturnCaptures, I, DT, IncludeI);
  PointerMayBeCaptured(V, &CB);
  return CB.Captured;
}

/// *TODO:* Write a new FunctionPass AliasAnalysis so that it can keep
/// a cache. Then we can move the code from BasicAliasAnalysis into
/// that path, and remove this threshold.
static int const Threshold = 20;
---

The whole thing was created in the first place with an explicit "shoring-up
a deficiency" and
"just wasting compile time" warning. There's surely a lot to be improved
here...

FWIW, my intend here is only to make it waste less compile time.
It would be awesome to see someone tackle it and rewrite the whole of it.
In the
meantime my suggestion is that we make it less compile time aggressive. :-)

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150507/9302fbab/attachment.html>