[PATCH] Factor DataflowWorklist out of LiveVariables and UninitializedValues analyses

[Jordan Rose]

On Apr 25, 2014, at 10:43 , Artyom Skrobov <Artyom.Skrobov at arm.com> wrote:

> > Are you sure this is correct? One worklist treats the entry block as already analyzed, the other doesn't. One starts with no blocks enqueued, the other effectively has all blocks enqueued because of the iterators.
>  
> Well, the patch keeps all tests passing.
> How can we trigger the supposed difference in their behaviour?
>  
> Anyway, the version that “effectively has all blocks enqueued because of the iterators” had
>   worklist.enqueueSuccessors(&cfg.getEntry());
> called immediately upon the creation.
> What would be the point of enqueuing blocks onto a list that already has all blocks enqueued?

Well, it could have resulted in traversal in a different order, but it doesn't. I'm not sure that entry actually does anything, since all blocks will be enqueued at the time. I wonder if this means we're using a less efficient traversal order than Ted intended...it wouldn't be the first time we've updated the same code multiple times and accidentally broken the algorithm. :-(

I guess I'm not comfortable applying this until we know what the intended traversal is for each analysis. Unfortunately Ted (being a manager) probably will not have time to look at this until after WWDC, i.e. another month from now.


>  > Why doesn't enqueueSuccessors use enqueueBlock?
>  
> Thanks for spotting that! Attaching the updated patch.
>  
> > Since this is only used by classes in the same component of Clang, it might make sense to put even the header in the lib/ directory. That way it doesn't show up when other people build tools on top of Clang. Then again, it is a generally reusable component.
>  
> Yes, I think it is as generally reusable as PostOrderCFGView it builds upon (which is, similarly, only used by classes in the same component).

Fair enough.

Jordan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140428/43ac4947/attachment.html>