Bug 122925 – Transition remaining op_get* JITStubs to JIT operations

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

[Mark Lam]

Transitioning:
    cti_op_get_by_id_generic
    cti_op_get_by_val
    cti_op_get_by_val_generic
    cti_op_get_by_val_string

[Mark Lam]

Created attachment 214422 [details]
the patch.

Turns out the failures I saw wile testing this change were due to https://bugs.webkit.org/show_bug.cgi?id=122938.  With 122938 now fixed, this patch passes the javascriptcore tests.

[Mark Lam]

Comment on attachment 214422 [details]
the patch.

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

> Source/JavaScriptCore/jit/JITOperations.cpp:1344
> +EncodedJSValue JIT_OPERATION operationGetByValDefault(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript)

I'm not sure that "operationGetByValDefault" is the best name for this function.  I had to rename it because there's already a DFG operationGetByVal.  Looking at the code, this function seems to be a more generic get_by_val then operationGetByValGeneric.  As such, I was not able to tease out why the original JITStub author chose the naming scheme for these get_by_val inline cache implementations.  Please suggest a better name if you have one.  Thanks.

[Geoffrey Garen]

Comment on attachment 214422 [details]
the patch.

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

r=me

>> Source/JavaScriptCore/jit/JITOperations.cpp:1344
>> +EncodedJSValue JIT_OPERATION operationGetByValDefault(ExecState* exec, EncodedJSValue encodedBase, EncodedJSValue encodedSubscript)
> 
> I'm not sure that "operationGetByValDefault" is the best name for this function.  I had to rename it because there's already a DFG operationGetByVal.  Looking at the code, this function seems to be a more generic get_by_val then operationGetByValGeneric.  As such, I was not able to tease out why the original JITStub author chose the naming scheme for these get_by_val inline cache implementations.  Please suggest a better name if you have one.  Thanks.

I think the naming here is fine, since this is the default function we call before any inline caching.

I think the fundamental issue here is that we need to consolidate helper functions between the DFG, the baseline JIT, and the LLInt -- now that they all use the C calling convention.

[Mark Lam]

Thanks for the review.  Landed in r157559: <http://trac.webkit.org/r157559>.