| Summary: | FTL should have an inefficient but correct implementation of GetById | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||
| Component: | JavaScriptCore | Assignee: | Filip Pizlo <fpizlo> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | atrick, barraclough, ggaren, mark.lam, mhahnenberg, msaboff, nrotem, oliver, sam | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 122739 | ||||||
| Attachments: |
|
||||||
|
Description
Filip Pizlo
2013-10-13 21:57:22 PDT
Created attachment 214126 [details]
the patch
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()]))); Comment on attachment 214126 [details]
the patch
r=me
(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! Landed in http://trac.webkit.org/changeset/157409 |