[webkit-reviews] review denied: [Bug 115138] [QNX][ARM] Use hardware integer division where available : [Attachment 199614] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Cosmin Truta
<ctruta at gmail.com>'s request for review:
Bug 115138: [QNX][ARM] Use hardware integer division where available
https://bugs.webkit.org/show_bug.cgi?id=115138

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

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


Better but there is still some confusions with your flags.

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.cpp:49
> +namespace JSC {
> +
> +static bool isIntegerDivSupported()
> +{
> +#if CPU(APPLE_ARMV7S)
> +    return true;
> +#elif OS(QNX) && defined(ARM_CPU_FLAG_IDIV)
> +    return !!(SYSPAGE_ENTRY(cpuinfo)->flags & ARM_CPU_FLAG_IDIV);
> +#else
> +    return false;
> +#endif
> +}
> +
> +const bool MacroAssemblerARMv7::s_isIntegerDivSupported =
isIntegerDivSupported();

This whole code could be in if OS(QNX).
You don't need ARMV7S and the fallback.

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:1921
> +
> +    static const bool s_isIntegerDivSupported;

This should be in #if OS(QNX)