Bug 133105

Summary: [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
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, mark.lam, mhahnenberg, msaboff, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 130756    
Attachments:
Description Flags
work in progress
none
the patch msaboff: review+

Description Filip Pizlo 2014-05-19 20:52:57 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2014-05-19 20:55:18 PDT
Created attachment 231750 [details]
work in progress
Comment 2 Filip Pizlo 2014-05-20 10:46:49 PDT
Created attachment 231776 [details]
the patch
Comment 3 Michael Saboff 2014-05-20 12:02:40 PDT
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
Comment 4 Filip Pizlo 2014-05-20 12:07:48 PDT
(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
Comment 5 Filip Pizlo 2014-05-20 17:45:56 PDT
Landed in http://trac.webkit.org/changeset/169143