[Webkit-unassigned] [Bug 68316] DFG JIT does not have full block-local CSE

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=68316





--- Comment #10 from Filip Pizlo <fpizlo at apple.com>  2011-09-17 17:25:43 PST ---
> > Source/JavaScriptCore/dfg/DFGNode.h:176
> > +    macro(PutByVal, NodeMustGenerate | NodeClobbersWorld) \
> > +    macro(PutByValAlias, NodeMustGenerate | NodeClobbersWorld) \
> > +    macro(GetById, NodeResultJS | NodeMustGenerate | NodeClobbersWorld) \
> > +    macro(PutById, NodeMustGenerate | NodeClobbersWorld) \
> > +    macro(PutByIdDirect, NodeMustGenerate | NodeClobbersWorld) \
> 
> Do we really need to consider the gets to clobber the world?  if we know that the get is a pure object property, then the access is pure

Currently we never predict or speculate the purity of GetById.  All GetById's get compiled with a slow path that may call arbitrary JS code for getters, or arbitrary C++ code in WebCore.

This will change once we start doing reasonable things for GetById.  We will still have a GetById that covers the cases where we know nothing (in which case it'll be have as it does now, and we'll still have to assume that it clobbers the world), but we will also have another GetById op that already knows the structure it's accessing and already knows that the access is pure.  That's probably my next task...

> 
> > Source/JavaScriptCore/dfg/DFGPropagator.cpp:732
> > +        case ArithAbs:
> 
> Don't we support sqrt(), and min/max?

Haven't landed that patch yet. :-)

> 
> In the longer term it seems we would probably want a generic handler for pure intrinsics.

pureCSE() is that handler.  So it's just a question of whether we want performNodeCSE() to have one switch statement (which is more efficient) or have an if statement at the top like:

if (node.op & NodeIsPure) {
    setReplacement(pureCSE(node));
    return;
}

I think that would be reasonable if the number of pure arithmetic nodes that are CSE-able grows really big.

> 
> > Source/JavaScriptCore/dfg/DFGPropagator.cpp:-443
> > -#if ENABLE(DFG_DEBUG_VERBOSE)
> > -    graph.dump(codeBlock);
> > -#endif
> 
> is this deliberate?

Yup, since fixpoint() now does dump() calls, including one at the end.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.