[Webkit-unassigned] [Bug 162124] op_get_by_id_with_this should use inline caching

[bugzilla-daemon at webkit.org]

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

--- Comment #39 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 293804
  --> https://bugs.webkit.org/attachment.cgi?id=293804
Patch

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

> Source/JavaScriptCore/bytecode/PolymorphicAccess.cpp:833
>      const Identifier& ident = *state.ident;

Have you tested this code when we have the result/thisGPR be aliased to each other? What about when an exception is thrown and they're aliased and you're in the DFG/FTL? You should prove to yourself that these work, and also write test cases that stress them (at least for now, since register allocation may change in the future)

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:4370
> +        base.use();
> +        thisValue.use();

This is not correct, we may need these if we throw an exception. So we're not yet done using these inputs.
(I know this is done in other places, and those places are also wrong).

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4338
> +        notCellList.append(m_jit.branchIfNotCell(JSValueRegs(baseGPR)));
> +        notCellList.append(m_jit.branchIfNotCell(JSValueRegs(thisValueGPR)));

Can you add modes to this node where we have CellUse for child1 and child2? Seems like this will be the common case, and the DFG may have already proven that these are cells.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:2855
> +        m_out.branch(
> +            isCell(base, provenType(m_node->child1())), unsure(cellCase), unsure(notCellCase));

What about child2() being a cell? Also, see above comment about having a form where we have CellUse for these things.

> Source/JavaScriptCore/jit/Repatch.cpp:347
> +void repatchGetByID(ExecState* exec, JSValue baseValue, const Identifier& propertyName, const PropertySlot& slot, StructureStubInfo& stubInfo, GetByIDKind kind, FunctionPtr repatchCallOperation)

I would add a new GetByIDKind and change the appropriateGenerecGetByIdFunction instead of changing this method to take an extra parameter.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161106/7c3227ee/attachment.html>