On Apr 25, 2014, at 10:43 , Artyom Skrobov <artyom.skro...@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
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
