Bug 133105 – [ftlopt] DFG bytecode parser should turn GetById with nothing but a Getter stub as stuff+handleCall, and handleCall should be allowed to inline if it wants to

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 231750 [details]
work in progress

[Filip Pizlo]

Created attachment 231776 [details]
the patch

[Michael Saboff]

Comment on attachment 231776 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231776&action=review

r=me provided you address the two issues raised in GetByIdStatus::computeForStubInfo().

> Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:127
> +    State takesSlowPath = TakesSlowPath;

The name takesSlowPath is confusing since it can have the value MakesCalls.

> Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:132
> +        for (unsigned i = 0; i < list->size(); ++i)
> +            takesSlowPath = MakesCalls;

Why is this a loop?  Are you missing "if (list->at(i).doesCalls())"?

> Source/JavaScriptCore/jit/JIT.h:245
> +        

Extraneous space - delete

[Filip Pizlo]

(In reply to comment #3)
> (From update of attachment 231776 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231776&action=review
> 
> r=me provided you address the two issues raised in GetByIdStatus::computeForStubInfo().
> 
> > Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:127
> > +    State takesSlowPath = TakesSlowPath;
> 
> The name takesSlowPath is confusing since it can have the value MakesCalls.

Renamed to slowPathState.

> 
> > Source/JavaScriptCore/bytecode/GetByIdStatus.cpp:132
> > +        for (unsigned i = 0; i < list->size(); ++i)
> > +            takesSlowPath = MakesCalls;
> 
> Why is this a loop?  Are you missing "if (list->at(i).doesCalls())"?

Yes!  Thanks!

> 
> > Source/JavaScriptCore/jit/JIT.h:245
> > +        
> 
> Extraneous space - delete

Removed

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/169143