[Webkit-unassigned] [Bug 160588] Refactor MathIC compilation process in Baseline and DFG to turn temporary registers usage more flexible

[bugzilla-daemon at webkit.org]

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

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

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

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3434
>> +    ASSERT(scratchFPRRegCount <= MATH_IC_MAX_FPR);
> 
> Please make this a static_assert

Ok

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3512
>> +    // we need to check if it is possible allocate new registers
> 
> It seems like all 32-bit snippets are able to use tags as their scratch, can we just remove this code?

My intention is to enable allocate scratches and avoid use tagGPRs to avoid instructions to restore the tags. Current Snnipets uses just one scratch that is mapped to result .tagGPR , but others like JITPowGenerator can use more. If scratches are already allocated, we don't need to use x/y.tagGPR and consequently, we don't need to restore their tags on snnipets.

>> Source/JavaScriptCore/jit/JITMathIC.h:56
>> +#define MATH_IC_MAX_FPR 1
> 
> I would make these static fields inside JITMathIC instead of a define.

Good. I am going to do that.

>> Source/JavaScriptCore/jit/JITMulGenerator.cpp:41
>> +        m_scratchGPR = m_result.tagGPR();
> 
> Have you made sure it's always valid to do this?

The current code always  use result.tagGPR as scratch on 32bits. So I suppose that it is valid. Also, AFIK all results assign are boxed, so the tag is going to be restored properly. I can double check that.

-- 
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/20161004/41bbbaeb/attachment.html>