[Webkit-unassigned] [Bug 162125] Enable IC for put_by_id_with_this

[bugzilla-daemon at webkit.org]

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

--- Comment #29 from Caio Lima <ticaiolima at gmail.com> ---
Comment on attachment 308242
  --> https://bugs.webkit.org/attachment.cgi?id=308242
Proposed Patch

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

>> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:1926
>> +#endif
> 
> Style: This seems slightly too tricky. I'd just have this be its own case, with a "break" both for 64/32 bit platforms.

Done.

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:249
>> +void SpeculativeJIT::cachedPutByIdWithThis(CodeOrigin codeOrigin, GPRReg baseGPR, GPRReg valueGPR, GPRReg thisGPR, GPRReg scratchGPR, unsigned identifierNumber, JITCompiler::JumpList slowPathTargets, SpillRegistersMode spillMode)
> 
> There is only one caller here, which never provides spillmode, please remove the parameter. Also, since this is a function w/ a single caller, I'd just remove this function alltogether and just inline it at the only call site.

Sounds better.

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:2944
>> +            putByIdWithThis(lowJSValue(m_node->child3()), lowCell(m_node->child1()), lowCell(m_node->child2()));
> 
> You unconditionally speculate CellUse in FixupPhase, but then check here if it's not CellUse. Perhaps FixupPhase's speculation should be conditional? Otherwise, that "else" branch is most likely dead code.

Oops. Removed from DFG and forgot it here.

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