[webkit-reviews] review granted: [Bug 125972] Add an utility class to simplify generating function calls : [Attachment 219604] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 125972: Add an utility class to simplify generating function calls
https://bugs.webkit.org/show_bug.cgi?id=125972

Attachment 219604: Patch
https://bugs.webkit.org/attachment.cgi?id=219604&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=219604&action=review


r=me

> Source/WebCore/ChangeLog:13
> +	   the stack, do the call, restores the stack, and restore the
registers.

Should be "does the call".

> Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:1148
> +    void test32AndSetFlags(RegisterID reg, TrustedImm32 mask =
TrustedImm32(-1))

Is there going to be a version of test32 that doesn't set flags? If not, then I
would just call this "test32".

> Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:1158
> +    Jump branchOnFlags(ResultCondition cond)

I would just call this "branch".

> Source/WebCore/cssjit/FunctionCall.h:41
> +    static const JSC::MacroAssembler::RegisterID firstArgument =
JSC::X86Registers::edi;
> +    static const JSC::MacroAssembler::RegisterID returnRegister =
JSC::X86Registers::eax;

Please use the values defined in GPRInfo (argumentGPR0, returnValueGPR) instead
of redefining them here.