[Webkit-unassigned] [Bug 233474] [JSC] Generated code size reductions for baseline JIT (all architectures)

[bugzilla-daemon at webkit.org]

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

Yusuke Suzuki <ysuzuki at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ysuzuki at apple.com
 Attachment #445419|review?, commit-queue?      |review+, commit-queue-
              Flags|                            |

--- Comment #4 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 445419
  --> https://bugs.webkit.org/attachment.cgi?id=445419
Patch

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

Nice. r=me.

> Source/JavaScriptCore/assembler/ARMv7Assembler.h:1268
> +        ASSERT(!(offset & 0x3));

According to ARM Architecture Reference Manual, https://developer.arm.com/documentation/ddi0406/b/Application-Level-Architecture/Instruction-Details/Alphabetical-list-of-instructions/LDRD--register-?lang=en
<Rt>
The first destination register. This register must be even-numbered and not R14.

<Rt2>
The second destination register. This register must be <R(t+1)>.

So, let's add assertions for the above condition.
ASSERT(rt2 == rt + 1);
ASSERT(rt % 2 == 0);
ASSERT(rt != r14);

> Source/JavaScriptCore/assembler/ARMv7Assembler.h:1745
> +        ASSERT(!(offset & 0x3));

Ditto.

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:62
> +    RegisterID scratchRegister() { return addressTempRegister; }

Can you review all places using this scratchRegister? I think it is possible that some places are using addressTempRegister and scratchRegister(), and in that case, this change breaks it.

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:872
> +        if (!(absOffset & ~0x3fc))
> +            m_assembler.ldrd(dest1, dest2, address.base, address.offset, /* index: */ true, /* wback: */ false);

According to ARM Architecture Reference Manual, we need to ensure that,
dest1 + 1 == dest2
dest1 != r14

> Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h:1009
> +            m_assembler.strd(src1, src2, address.base, address.offset, /* index: */ true, /* wback: */ false);

Ditto.

> Source/JavaScriptCore/jit/AssemblyHelpers.h:207
> +        static_assert((PayloadOffset == 4 && !TagOffset) || (!PayloadOffset && TagOffset == 4));
> +        if constexpr (PayloadOffset < TagOffset)
> +            loadPair32(address, regs.payloadGPR(), regs.tagGPR());
> +        else
> +            loadPair32(address, regs.tagGPR(), regs.payloadGPR());

Ditto.

> Source/JavaScriptCore/jit/AssemblyHelpers.h:216
> +        static_assert((PayloadOffset == 4 && !TagOffset) || (!PayloadOffset && TagOffset == 4));

You can insert static_assert that,

static_assert(!PayloadOffset && TagOffset == 4)

because we never enable JIT on BigEndian environments.

> Source/JavaScriptCore/jit/AssemblyHelpers.h:220
> +        if constexpr (PayloadOffset < TagOffset)
> +            loadPair32(address, regs.payloadGPR(), regs.tagGPR());
> +        else
> +            loadPair32(address, regs.tagGPR(), regs.payloadGPR());

And we can just use loadPair32(address, regs.payloadGPR(), regs.tagGPR());

> Source/JavaScriptCore/jit/JITOpcodes.cpp:1260
> -    for (size_t j = CodeBlock::llintBaselineCalleeSaveSpaceAsVirtualRegisters(); j < count; ++j)
> -        emitInitRegister(virtualRegisterForLocal(j));
> +    size_t first = CodeBlock::llintBaselineCalleeSaveSpaceAsVirtualRegisters();
> +    if (first < count)
> +        moveTrustedValue(jsUndefined(), jsRegT10);
> +    for (size_t j = first; j < count; ++j)
> +        emitPutVirtualRegister(virtualRegisterForLocal(j), jsRegT10);

Probably, we should use ENABLE(EXTRA_CTI_THUNKS) path in all architectures.
But for now, it is OK since op_enter_handlerGenerator assumes 64bit right now.

-- 
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/20211201/ce2e6c7f/attachment-0001.htm>