<div dir="ltr"><br><br>On Tue, May 5, 2015 at 7:10 PM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org">dberlin@dberlin.org</a>> wrote:<br>> As an incremental solution, this looks like a good idea.<br>><br>> However, overall, this whole thing is, IMHO, the wrong solution.<br>> Trying to compute a stateful problem with a stateless API is always<br>> going to be slow :)<br>> Caching does not really fix this.<br><br>Yep<br><br>> The right way is to compute capturing as a stateful constraint/other problem,<br>> once,  and if we discover we invalidate info too much, compute it again.<br>><br>> This is like calling it "lazy value info" and then computing it on<br>> every variable, 5 times :)<br>><br>> At the point at which you compute it for everything, it's no longer<br>> lazy, and it doesn't make a whole lot of sense to try to treat it as a<br>> lazy problem.<br><br>True! Take a look at some comments:<div><br><div>lib/Analysis/AliasAnalysis.cpp:<br>---<br>// <b>FIXME:</b> this is really just shoring-up a deficiency in alias analysis.<br>// BasicAA isn't willing to spend linear time determining whether an alloca<br>// was captured before or after this particular call, while we are. However,<br>// with a smarter AA in place, this test is just wasting compile time.<br>AliasAnalysis::ModRefResult<br>AliasAnalysis::<b>callCapturesBefore</b>(const Instruction *I,<br>                                  const AliasAnalysis::Location &MemLoc,<br>                                  DominatorTree *DT) {<br>  if (!DT)<br>    return AliasAnalysis::ModRef;<br><br>  const Value *Object = GetUnderlyingObject(MemLoc.Ptr, *DL);<br>  if (!isIdentifiedObject(Object) || isa<GlobalValue>(Object) ||<br>      isa<Constant>(Object))<br>    return AliasAnalysis::ModRef;<br>      <br>  ImmutableCallSite CS(I);<br>  if (!CS.getInstruction() || CS.getInstruction() == Object)<br>    return AliasAnalysis::ModRef;<br>        <br>  if (llvm::<b>PointerMayBeCapturedBefore</b>(Object, /* ReturnCaptures */ true,<br>                                       /* StoreCaptures */ true, I, DT,<br>                                       /* include Object */ true))<br>...</div><div>---</div><div><br></div><div>lib/Analysis/CaptureTracking.cpp<br></div><div>---<br>bool llvm::<b>PointerMayBeCapturedBefore</b>(const Value *V, bool ReturnCaptures,<br>                                      bool StoreCaptures, const Instruction *I,<br>                                      DominatorTree *DT, bool IncludeI) {<br>  assert(!isa<GlobalValue>(V) &&<br>         "It doesn't make sense to ask whether a global is captured.");<br><br>  if (!DT)<br>    return PointerMayBeCaptured(V, ReturnCaptures, StoreCaptures);<br><br>  // <b>TODO:</b> See comment in PointerMayBeCaptured regarding what could be done<br>  // with StoreCaptures.<br><br>  CapturesBefore CB(ReturnCaptures, I, DT, IncludeI);<br>  PointerMayBeCaptured(V, &CB);<br>  return CB.Captured;<br>}<br><br>/// <b>TODO:</b> Write a new FunctionPass AliasAnalysis so that it can keep<br>/// a cache. Then we can move the code from BasicAliasAnalysis into<br>/// that path, and remove this threshold.<br>static int const Threshold = 20;<br>---<br><br>The whole thing was created in the first place with an explicit "shoring-up a deficiency" and<br>"just wasting compile time" warning. There's surely a lot to be improved here... <br><br>FWIW, my intend here is only to make it waste less compile time.<br>It would be awesome to see someone tackle it and rewrite the whole of it. In the<br>meantime my suggestion is that we make it less compile time aggressive. :-)<br><br>-- <br>Bruno Cardoso Lopes<br><a href="http://www.brunocardoso.cc">http://www.brunocardoso.cc</a></div></div></div>