[webkit-reviews] review granted: [Bug 122925] Transition remaining op_get* JITStubs to JIT operations : [Attachment 214422] the patch.

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 122925: Transition remaining op_get* JITStubs to JIT operations
https://bugs.webkit.org/show_bug.cgi?id=122925

Attachment 214422: the patch.
https://bugs.webkit.org/attachment.cgi?id=214422&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
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.