[PATCH] Factor DataflowWorklist out of LiveVariables and UninitializedValues analyses

[Artyom Skrobov]

Thank you Jordan - I see that it would be safest to wait a month, until Ted
is available to clarify how his code is intended to work.

 

 

From: Jordan Rose [mailto:jordan_rose at apple.com] 
Sent: 28 April 2014 17:30
To: Artyom Skrobov
Cc: Ted Kremenek; cfe-commits at cs.uiuc.edu
Subject: Re: [PATCH] Factor DataflowWorklist out of LiveVariables and
UninitializedValues analyses

 

 

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/20140429/44322bce/attachment.html>