[Webkit-unassigned] [Bug 84553] DFG should have control flow graph simplification
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Apr 27 10:50:26 PDT 2012
https://bugs.webkit.org/show_bug.cgi?id=84553
Oliver Hunt <oliver at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #139140|review? |review+
Flag| |
--- Comment #15 from Oliver Hunt <oliver at apple.com> 2012-04-27 10:50:26 PST ---
(From update of attachment 139140)
View in context: https://bugs.webkit.org/attachment.cgi?id=139140&action=review
r=me, but make sure we don't have any custom toBoolean(CallFrame*) overrides in WebCore -- I recall that there was one, something like document.all -- but it's possible we hanlde that with the MasqueradesAsUndefined flag.
r+ with the few changes i suggested.
> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:153
> + root->valuesAtHead.local(i).makeTop();
Sadness. Maybe one day we'll be able to do some analysis of capture sites. Oh well.
> Source/JavaScriptCore/dfg/DFGAdjacencyList.h:45
> + #define AdjacencyListSize 3
could we use enum { AdjacencyListSize = 3 } rather than a #define here? Aside from anything else #define's are hard to see in the debugger. (i'd suggest static const but i see we use it as an array length below. Silly C++ const expr requirements.
> Source/JavaScriptCore/dfg/DFGEdge.h:83
> + // use an Edge where you meant to use Edge::index(), you will instead get Edge::isSet(). Gross!
There's a horrible trick we use in other parts of the code where you can do this:
typedef void* (DFGEdge::*UnspecifiedBoolType);
operator UnspecifiedBoolType*() const { return isSet ? reinterpret_cast<UnspecifiedBoolType*>(1) : 0; }
This gives you (essentially) operator bool() but without the horrible implied int conversions.
> Source/JavaScriptCore/dfg/DFGGraph.cpp:130
> - if (mustGenerate) {
> - ASSERT(refCount);
> + if (mustGenerate)
> --refCount;
Seems reasonable to keep this assertion.
> Source/JavaScriptCore/dfg/DFGNode.h:478
> + return 1;
> + case Branch:
> + return 2;
Mmmmmm tasty constants ;) (not saying that there's anything better though :-(
> Source/JavaScriptCore/heap/Heap.cpp:860
> double lastGCEndTime = WTF::currentTime();
> m_lastGCLength = lastGCEndTime - lastGCStartTime;
> JAVASCRIPTCORE_GC_END();
> -
> +
> (*m_activityCallback)();
> }
>
revert Heap.cpp?
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list