[Webkit-unassigned] [Bug 63409] DFG JIT does not perform put_by_id caching

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63409


Gavin Barraclough <barraclough at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #98642|review?                     |review-
               Flag|                            |




--- Comment #4 from Gavin Barraclough <barraclough at apple.com>  2011-06-27 12:00:56 PST ---
(From update of attachment 98642)
View in context: https://bugs.webkit.org/attachment.cgi?id=98642&action=review

Hey Filip, this patch looks great, and I'd be happy to accept as is.  I've r-ed to bounce back to you to give you a chance to look at a couple of suggestions, but feel free to consider both optional, and just reset r? on this patch if you want.  Our coding style leans against bool parameters where possible, since their meaning at the call site is often opaque, in many cases we prefer enumerated types.  In this case I'm not sure that I'd recommend adding an extra enum, so I'd either pass a function pointer or stick with the bool.

> Source/JavaScriptCore/dfg/DFGJITCodeGenerator.cpp:366
> +    if (m_jit.codeBlock()->isStrictMode()) {

Given that 'direct' is only used to select the operation to call, I think it might make the code a little clearer to make this function take a V_DFGOperation_EJJI rather than a bool.  It would remove this chunk of code, and make the call site a little more transparent (the meaning of bool arguments isn't always obvious).  Obviously each call site would still need to do something like "m_jit.codeBlock()->isStrictMode() ? operationPutByIdDirectStrictOptimize : operationPutByIdDirectNonStrictOptimize".  What do you think?

> Source/JavaScriptCore/dfg/DFGRepatch.cpp:138
> +void dfgRepatchPutByID(ExecState* exec, JSValue baseValue, const Identifier& propertyName, const PutPropertySlot& slot, StructureStubInfo& stubInfo, bool direct)

Similarly to during compilation, again you could switch the bool argument to a V_DFGOperation_EJJI, and ditch the appropriatePutByIdFunction method.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.