[Webkit-unassigned] [Bug 183130] [MIPS] Optimize generated JIT code for branches

[bugzilla-daemon at webkit.org]

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

Yusuke Suzuki <utatane.tea at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #335174|review?                     |review-
              Flags|                            |

--- Comment #8 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 335174
  --> https://bugs.webkit.org/attachment.cgi?id=335174
MIPS branches

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

> Source/JavaScriptCore/ChangeLog:15
> +        on a MIPS platform. Firstly, the patch https://bugs.webkit.org/show_bug.cgi?id=101328
> +        added two nop instructions to branchEqual() and branchNotEqual() in order to prevent
> +        the beq/bne instruction in a patchable branch from being overwritten by replaceWithJump()
> +        in case when we have to jump outside the 256MB-aligned block. However, this adds a significant
> +        overhead for all other types of branches. Since these nop's protect the code that comes after
> +        the code generated by moveWithPatch(), this function seems like a better place to add them.
> +        Secondly, in branches with immediate operand, the immediate value is 0 in many cases, and in

What happens if we just use patchableBranch64 and repatch it?
I don't think this is safe if it becomes broken.
How about separating patchableBranchPtr, patchableBranchPtrWithPatch etc. from branchPtr implementation and emitting nops in patchable versions?

> Source/JavaScriptCore/assembler/MIPSAssembler.h:1016
> +        cacheFlush(insn - 2, codeSize);

Lol, nice. I think `cacheFlush(instructionStart, codeSize);` is nicer.
BTW, replaceWithLoad and replaceWithAddressComputation should be fixed at least. Could you review all the other replacing functions too?

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1834
>              */

Let's change the comment.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1887
>              */

Ditto.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1961
>              if (imm.m_value >= -32768 && imm.m_value  <= 32767 && !m_fixedWidth) {

Ditto.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:2025
>              m_assembler.mfhi(dataTempRegister);

Ditto.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:2070
>              m_assembler.mfhi(dataTempRegister);

Ditto.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:2120
>              */

Ditto.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:2179
>              */

Ditto.

-- 
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/20180320/42b8e7d1/attachment.html>