I think this will make the code easier cleaner, instead of just having comments stating that a particular field is only used in SSA.
The vector called m_entrypoints now I'm going to rename to m_roots. This represents the CFG roots. I'll also rename m_entrypointToArguments to m_rootToArguments. Similarly, I'll rename Graph::isEntrypoint to Graph::isRoot m_numberOfEntrypoints is only used in SSA, and I'll keep the same name. That field represents the number of logical entrypoints this compilation is making. It's not necessarily the number of CFG roots.
Created attachment 319930 [details] patch
Comment on attachment 319930 [details] patch r=me
Comment on attachment 319930 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=319930&action=review r=me too. > Source/JavaScriptCore/ChangeLog:17 > + You should probably say that you renamed m_entrypoints to m_roots in the changelog.
Comment on attachment 319930 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=319930&action=review r=me. The code changes seem fine. Please update the ChangeLog. > Source/JavaScriptCore/ChangeLog:16 > + Graph::m_numberOfEntrypoints is only used in SSA when compiling with > + EntrySwitch. It represents the logical number of entrypoints the compilation > + will end up with. Crucially, Graph::m_numberOfEntrypoints != Graph::m_roots.size(). > + This is the main reason I've renamed the fields and related methods. I don't find changes to m_numberOfEntrypoints in the patch. It would be good to discuss the reasoning behind the change of m_entrypointToArguments to m_rootToArguments.
Comment on attachment 319930 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=319930&action=review >> Source/JavaScriptCore/ChangeLog:16 >> + This is the main reason I've renamed the fields and related methods. > > I don't find changes to m_numberOfEntrypoints in the patch. It would be good to discuss the reasoning behind the change of m_entrypointToArguments to m_rootToArguments. This patch doesn't change m_numberOfEntrypoints on purpose. That field is retaining its name because it describes its purpose. Also, this patch renames m_entrypointToArguments to m_rootToArguments to be consistent with the change of m_entrypoints to m_roots. >> Source/JavaScriptCore/ChangeLog:17 >> + > > You should probably say that you renamed m_entrypoints to m_roots in the changelog. It says this above.
New changelog: This patch does renaming to make the distinction between Graph::m_entrypoints and Graph::m_numberOfEntrypoints more clear. The source of confusion is that Graph::m_entrypoints.size() is not equivalent to Graph::m_numberOfEntrypoints. Graph::m_entrypoints is really just the CFG roots. In CPS, this vector has size >= 1. In SSA, the size is always 1. This patch renames Graph::m_entrypoints to Graph::m_roots. To be consistent, this patch also renames Graph's m_entrypointToArguments field to m_rootToArguments. Graph::m_numberOfEntrypoints retains its name. This field is only used in SSA when compiling with EntrySwitch. It represents the logical number of entrypoints the compilation will end up with. Each EntrySwitch has m_numberOfEntrypoints cases.
Created attachment 319932 [details] patch for landing
Comment on attachment 319932 [details] patch for landing Clearing flags on attachment: 319932 Committed r221637: <http://trac.webkit.org/changeset/221637>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34693633>