[Webkit-unassigned] [Bug 183786] Implement setupArgumentsImpl for ARM and MIPS

[bugzilla-daemon at webkit.org]

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

Yusuke Suzuki <utatane.tea at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #338093|review?                     |review+, commit-queue-
              Flags|                            |

--- Comment #15 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 338093
  --> https://bugs.webkit.org/attachment.cgi?id=338093
Patch

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

r=me with nits. Nice work!

> Source/JavaScriptCore/ChangeLog:39
> +        * assembler/MacroAssemblerARMv7.h:
> +        (JSC::MacroAssemblerARMv7::moveDouble):
> +        * assembler/MacroAssemblerMIPS.h:
> +        (JSC::MacroAssemblerMIPS::moveDouble):
> +        * jit/CCallHelpers.h:
> +        (JSC::CCallHelpers::setupStubCrossArgs):
> +        (JSC::CCallHelpers::ArgCollection::ArgCollection):
> +        (JSC::CCallHelpers::ArgCollection::pushRegArg):
> +        (JSC::CCallHelpers::ArgCollection::pushExtraRegArg):
> +        (JSC::CCallHelpers::ArgCollection::addGPRArg):
> +        (JSC::CCallHelpers::ArgCollection::addGPRExtraArg):
> +        (JSC::CCallHelpers::ArgCollection::addStackArg):
> +        (JSC::CCallHelpers::ArgCollection::addPoke):
> +        (JSC::CCallHelpers::ArgCollection::argCount):
> +        (JSC::CCallHelpers::clampArrayToSize):
> +        (JSC::CCallHelpers::calculatePokeOffset):
> +        (JSC::CCallHelpers::pokeForArgument):
> +        (JSC::CCallHelpers::pokeAligned):
> +        (JSC::CCallHelpers::marshallArgumentRegister):
> +        (JSC::CCallHelpers::setupArgumentsImpl):
> +        (JSC::CCallHelpers::align2):
> +        (JSC::CCallHelpers::pokeArgumentsAligned):
> +        (JSC::CCallHelpers::std::is_integral<CURRENT_ARGUMENT_TYPE>::value):
> +        (JSC::CCallHelpers::std::is_pointer<CURRENT_ARGUMENT_TYPE>::value):
> +        (JSC::CCallHelpers::setupArguments):
> +        * jit/FPRInfo.h:
> +        (JSC::FPRInfo::toArgumentRegister):

Could you update this ChangeLog to reflect the change in the latest patch? You can run `Tools/Scripts/webkit-patch upload --update-changelogs`.

> Source/JavaScriptCore/jit/CCallHelpers.h:585
> +            auto updatedArgSourceRegs1 = argSourceRegs.pushRegArg(arg.payloadGPR(), GPRInfo::toArgumentRegister(alignedArgCount));
> +            auto updatedArgSourceRegs2 = updatedArgSourceRegs1.pushExtraRegArg(arg.tagGPR(), GPRInfo::toArgumentRegister(alignedArgCount + 1));

OK, so can you add a comment about why we are increasing extra reg arg and not increasing numGPRArgs by 2 instead? It is tricky if we know that JSValueRegs is represented by two GPRRegs in 32bit function calls.

> Source/JavaScriptCore/jit/CCallHelpers.h:674
> +#if CPU(JSVALUE64)
>          static_assert(FunctionTraits<OperationType>::cCallArity() == numGPRArgs + numFPRArgs + extraPoke, "Check the CCall arity");
> +#endif

So, can we keep this static_assert for CPU(X86) too? It is useful for keeping 32bit build sane.

-- 
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/20180417/16f36e6e/attachment.html>