[Webkit-unassigned] [Bug 196080] Support more paren expressions in RegExp-JIT on ARM/MIPS

[bugzilla-daemon at webkit.org]

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

Guillaume Emont <guijemont at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |guijemont at igalia.com

--- Comment #8 from Guillaume Emont <guijemont at igalia.com> ---
Comment on attachment 365682
  --> https://bugs.webkit.org/attachment.cgi?id=365682
Patch

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

Did not test it, but the changes make sense to me, with a few nitpicks.

> Source/JavaScriptCore/assembler/MacroAssembler.h:585
> +    void addPtr(RegisterID src, Address address)

should address be called dest instead?

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:214
> +    void add32(RegisterID src, Address address)

Same question as for addPtr()

> Source/JavaScriptCore/yarr/YarrJIT.cpp:3785
> +        pushPair(ARMRegisters::r11, ARMRegisters::r4); // push r4 twice for stack alignment

Would it cost more to do something like this? (for clarity sake):
  push(ArmRegisters::r11);
  sub32(stackPointerRegister, 4);

> Source/JavaScriptCore/yarr/YarrJIT.cpp:3799
> +            move(TrustedImm32(0x10000), supplementaryPlanesBase);
> +            move(TrustedImm32(0xfffffc00), surrogateTagMask);
> +            move(TrustedImm32(0xd800), leadingSurrogateTag);
> +            move(TrustedImm32(0xdc00), trailingSurrogateTag);

shouldn't we have const variables or defines for these immediate values?

> Source/JavaScriptCore/yarr/YarrJIT.cpp:3851
> +        popPair(ARMRegisters::r11, ARMRegisters::r4); // pop r4 twice for stack alignment

see my comment when doing the matching pushPair().

> Source/WTF/wtf/Platform.h:1013
> +#if CPU(ARM64) || (CPU(X86_64) && !OS(WINDOWS)) || OS(LINUX) && (CPU(ARM_THUMB2) || CPU(MIPS))

maybe an added pair of parentheses could make the meaning more obvious:
#if CPU(ARM64) || (CPU(X86_64) && !OS(WINDOWS)) || (OS(LINUX) && (CPU(ARM_THUMB2) || CPU(MIPS)))

-- 
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/20190322/df6c0290/attachment.html>