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

[bugzilla-daemon at webkit.org]

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

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

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

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:969
> +        m_assembler.vmov(dest, RegisterID(dest+1), src);

Use `dest + 1`.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:2926
> +        m_assembler.mfc1(RegisterID(dest+1), FPRegisterID(src+1));

Ditto.

> Source/JavaScriptCore/jit/CCallHelpers.h:305
> +        RELEASE_ASSERT(TargetSize <= sourceArray.size());

Keep `static_assert`.

> Source/JavaScriptCore/jit/CCallHelpers.h:341
> +    ALWAYS_INLINE bool pokeAligned(unsigned currentGPRArgument, unsigned currentFPRArgument, unsigned numCrossSources, unsigned extraGPRArgs, unsigned extraPoke)

The function name seems confusing. It does not `poke`. Use the name like `shouldPokeAligned` or the other.

> Source/JavaScriptCore/jit/CCallHelpers.h:-337
> -#else // USE(JSVALUE64)

Let's keep this #else.

> Source/JavaScriptCore/jit/CCallHelpers.h:399
> +#elif CPU(X86)

And add it as `#if CPU(X86)`.

> Source/JavaScriptCore/jit/CCallHelpers.h:449
> +    ALWAYS_INLINE unsigned align2(unsigned val)
> +    {
> +        return (val + 1) & (~1);
> +    }

Use `roundUpToMultipleOf<2>(val)` instead and remove this function.

> Source/JavaScriptCore/jit/CCallHelpers.h:493
> +            pokeForArgument(arg, numGPRArgs, numFPRArgs, numCrossSources, extraGPRArgs+1, extraPoke);

Use `extraPoke + 1`.

> Source/JavaScriptCore/jit/CCallHelpers.h:503
> +            pokeForArgument(arg, numGPRArgs, numFPRArgs, numCrossSources, extraGPRArgs, extraPoke+1);

Use `extraPoke + 1`.

> Source/JavaScriptCore/jit/CCallHelpers.h:534
> +

Remove this blank line.

> Source/JavaScriptCore/jit/CCallHelpers.h:539
> +

Remove this blank line.

> Source/JavaScriptCore/jit/CCallHelpers.h:549
> +    setupArgumentsImpl(ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke> argSourceRegs, JSValue::JSCellTag, GPRReg payload, Args... args)

Now, `JSValue::JSCellTag` is removed. We are using `CCallHelpers::CellValue` instead.

> Source/JavaScriptCore/jit/CCallHelpers.h:563
> +            move(TrustedImm32(JSValue::CellTag), GPRInfo::toArgumentRegister(alignedArgCount+1));

Use `alignedArgCount + 1`.

> Source/JavaScriptCore/jit/CCallHelpers.h:564
> +

Remove this blank line.

> Source/JavaScriptCore/jit/CCallHelpers.h:571
> +    setupArgumentsImpl(ArgCollection<numGPRArgs, numGPRSources, numFPRArgs, numFPRSources, numCrossSources, extraGPRArgs, extraPoke> argSourceRegs, JSValue::JSCellTag, TrustedImmPtr payload, Args... args)

Ditto. But maybe this function is not necessary. We do not have `JSValue::JSCellTag & TrustedImmPtr` combination right now.

> Source/JavaScriptCore/jit/CCallHelpers.h:584
> +            move(TrustedImm32(JSValue::CellTag), GPRInfo::toArgumentRegister(alignedArgCount+1));

Ditto.

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

Use `alignedArgCount + 1`.
And is `pushExtraRegArg` correct? Do we always need extra reg arg to push JSValueRegs?

> Source/JavaScriptCore/jit/CCallHelpers.h:606
> +

Remove this blank line.

> Source/JavaScriptCore/jit/CCallHelpers.h:-453
> -#undef RESULT_TYPE

Do not remove this undef.

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

Add a valid static_assert for 32bit.

-- 
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/20180413/079da472/attachment-0001.html>