-
Notifications
You must be signed in to change notification settings - Fork 84
Incremental TD3: incrementalize analysis within functions #409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…t edges in comparison
…threaten uniquness of ids when updating them
jerhard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments on minor things. I did not have a deep look into the actual CFG-comparison yet. Maybe you can add some short comments there?
|
I added comments for the compare algorithm and renaming as you suggested in the comments. Also I added the separate compareCIL module which uses the compare functions either from CompareCFG or CompareAST based on the configuration. |
By that they get a new id and are reananalyzed for reluctant destabilization without any function-internal destabilization of primary obsolete nodes
…rn nodes in and incremental run
| ignore @@ Pretty.printf "test for %a\n" Node.pretty_trace (S.Var.node x); | ||
| solve x Widen; | ||
| if not (op (HM.find rho x) old_rho) then ( | ||
| print_endline "Destabilization required..."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this printout be there? Maybe use tracing instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only extends the existing printouts during the incremental preparation in TD3 to make them more comprehensible in case of reluctant destabilization. I guess one should change all of them such that they are only output in verbose mode or use some existing tracing functions...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for the time being it's better if they aren't tracing, but also usable normally. It makes interpreting and debugging the interactive benchmark logs easier to see what happened and what didn't.
So making them verbose-only would be more reasonable, but since the rest are normally printed right now anyway, I suppose this can stay for now.
Closes #351
This PR contains the refinement for the incremental analysis which allows to reuse previous results for the unchanged nodes in changed functions. Instead of differentiating only between changed, unchanged, removed and added functions, changed functions can now be compared more detailed based on their control flow graph to determine the
unchangedNodes,primObsoleteNodesandprimNewNodesin the function. The unchanged nodes are tuples of old and new nodes that match and that only have incoming edges from other matching nodes. The primary obsolete nodes are nodes from the old CFG for which no match could be found and that suffice to be destabilized in an incremental analysis run. That means, that all changed nodes can be reached from the primary obsolete nodes. The primary new nodes are the equivalent to the primary obsolete nodes in the new CFG.The changes of this PR include