[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 #22 from Saam Barati <sbarati at apple.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_GPR);
> +    ASSERT(scratchFPRRegCount <= MATH_IC_MAX_FPR);

Please make this a static_assert

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:3512
> +    // Since registers on 32-bit architectures like x86_32 are scarce
> +    // 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?

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:1569
> +        ASSERT(numGPScratchRegisters <= MATH_IC_MAX_GPR);
> +        ASSERT(numFPScratchRegisters <= MATH_IC_MAX_FPR);

Please make this a static_assert

> Source/JavaScriptCore/jit/JITMathIC.h:56
> +#define MATH_IC_MAX_GPR 1
> +#define MATH_IC_MAX_FPR 1

I would make these static fields inside JITMathIC instead of a define.

> Source/JavaScriptCore/jit/JITMulGenerator.cpp:41
> +    if (m_scratchGPR == InvalidGPRReg)
> +        m_scratchGPR = m_result.tagGPR();

Have you made sure it's always valid to do this?

-- 
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/815821c1/attachment.html>