Bug 122740 – FTL should have an inefficient but correct implementation of GetById

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

[Filip Pizlo]

Just to expand coverage and find bugs before I implement this for real.

[Filip Pizlo]

Created attachment 214126 [details]
the patch

[Nadav Rotem]

I see two changes: 1. remove prediction  2. implement getByID   Are these changes related ? 

What is J_JITOperation_EJI ?

The line below is really long. Can you extract the identifier number into a separate variable ?

 1231        setJSValue(vmCall(m_out.operation(operationGetById), m_callFrame, base, m_out.constIntPtr(m_graph.identifiers()[m_node->identifierNumber()])));

[Mark Hahnenberg]

Comment on attachment 214126 [details]
the patch

r=me

[Filip Pizlo]

(In reply to comment #2)
> I see two changes: 1. remove prediction  2. implement getByID   Are these changes related ? 

Loosely - I implemented the FTL version of GetById by looking at what the DFG version did, but of course removing all of the inline cache stuff.  Then I saw that the DFG had this assertion about predictions.  I had a choice of either adding such an assertion in the FTL, or removing it.  So I removed it.

> 
> What is J_JITOperation_EJI ?

It's a typedef:

typedef EncodedJSValue JIT_OPERATION (*J_JITOperation_EJI)(ExecState*, EncodedJSValue, StringImpl*);

See JITOperations.h.

> 
> The line below is really long. Can you extract the identifier number into a separate variable ?
> 
>  1231        setJSValue(vmCall(m_out.operation(operationGetById), m_callFrame, base, m_out.constIntPtr(m_graph.identifiers()[m_node->identifierNumber()])));

Ok!

[Filip Pizlo]

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