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

[Bruno Cardoso Lopes]

> I think that this is really nice.
>
> +bool isPotentiallyReachableInner(SmallVectorImpl<BasicBlock *> &Worklist,
>
> I'd prefer to rename this function if it will now be externally accessible: 'Inner' is not particularly descriptive.

Agreed!

> +      // Start the search with the instruction found in the last lookup round.
>
> How do you know that subsequent calls to find specify instructions that always come later in the BB than those from previous calls?

I'm not sure I follow your question, but if execution reaches this
point is because none of the instructions are already cached, that
said, they *have* to be further on in the BB - since the BB never
changes this guaranteed. The search will then resume from the last
point here an instruction was found and cached. Note that up to this
point, all previous instructions in the BB are already cached. Let me
know if I got your question right :-)

>
> +    /// It also does not consider the scenario in 'isPotentiallyReachable'
> +    /// where 'A' comes after 'B' but may dominate through 'BB' successors.
>
> What does this mean exactly? Are you saying that the caller isPotentiallyReachable handles this case itself?

Basically, if isPotentiallyReachable finds that A comes after B, it
calls isPotentiallyReachableInner in order to find if there's any path
that makes A reach B. The comment tried to explain that this method
won't do that, it will only give answers considering instructions in
the same BB. isPotentiallyReachable is called somewhere else. I guess
this comment is a bit misleading, gonna fix that.

>
> +        if (isa<InvokeInst>(BeforeHere) || isa<PHINode>(I) || I == BeforeHere)
> +          return false;
>
> Why do we return false when I == BeforeHere?

Because it's not safe to Prune the search. This maintains the old
behavior from shouldExplore before this patch, i.e., tt means
shouldExplore should return true.

It takes ~2min to compile though, still a long time to wait under -O1/-O2 =T
What if we incorporate the limited approach into this patch for opt
levels < -O3 and let it scan everything otherwise?

Thanks Hal,

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc