[webkit-reviews] review denied: [Bug 133596] make css jit work on armv7 : [Attachment 232713] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 133596: make css jit work on armv7
https://bugs.webkit.org/show_bug.cgi?id=133596

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

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=232713&action=review


> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:1415
> +	   // use addressTempRegister incase the branch32 we call uses
dataTempRegister. :-/

I don't understand the problem here. Why isn't addressTempRegister the right
register in this case?

> Source/WebCore/cssjit/FunctionCall.h:94
> +#elif CPU(ARM_THUMB2)

What about MIPS? ;)

> Source/WebCore/cssjit/RegisterAllocator.h:67
> +    JSC::ARMRegisters::r6,

We cannot use r6, it could be used for addressTempRegister.

I believe r4->r6 are callee saved registers (
https://developer.apple.com/library/ios/documentation/Xcode/Conceptual/iPhoneOS
ABIReference/Articles/ARMv6FunctionCallingConventions.html#//apple_ref/doc/uid/
TP40009021-SW10 )

> Source/WebCore/cssjit/SelectorCompiler.cpp:1095
> +    prologueRegisters.append(JSC::ARMRegisters::fp);

Do we care about fp?

> Source/WebCore/cssjit/SelectorCompiler.cpp:1138
> +    // ARM_THUMB2 does not have enough registers to compile complicated
selectors.
> +    // Compiling should always succeed on non-ARM_THUMB2 CPUs.
> +    ASSERT_NOT_REACHED();

Indent.