Hello Jordan and Ted,
Is it the right time now to look again at unifying the two DataflowWorklists? Last month, we've got as far as establishing that the DataflowWorklist in UninitializedValues may not have been intended to start with all blocks implicitly enqueued, since it gets some blocks enqueued onto it explicitly right after the creation; but we couldn't decide without Ted whether the difference in implementation of the two DataflowWorklists was intentional or not. From: Jordan Rose [mailto:[email protected]] Sent: 28 April 2014 17:30 To: Artyom Skrobov Cc: Ted Kremenek; [email protected] Subject: Re: [PATCH] Factor DataflowWorklist out of LiveVariables and UninitializedValues analyses On Apr 25, 2014, at 10:43 , Artyom Skrobov <[email protected]> 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
DataflowWorklist.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
