[Webkit-unassigned] [Bug 183740] [MIPS] Optimize JIT code generated by methods with TrustedImm32 operand

[bugzilla-daemon at webkit.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #336038|review?                     |review+
              Flags|                            |

--- Comment #4 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 336038
  --> https://bugs.webkit.org/attachment.cgi?id=336038
Patch

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

r=me with some comments.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:686
> -        if (imm.m_value == -1) {
> +        if (imm.m_value == -1 && !m_fixedWidth) {
>              m_assembler.nor(dest, dest, MIPSRegisters::zero);
>              return;
>          }
> -
> -        /*
> -            li  immTemp, imm
> -            xor dest, dest, immTemp
> -        */
> -        move(imm, immTempRegister);
> -        m_assembler.xorInsn(dest, dest, immTempRegister);
> +        if (imm.m_value >= 0 && imm.m_value <= 65535 && !m_fixedWidth)
> +            m_assembler.xori(dest, dest, imm.m_value);
> +        else {
> +            /*
> +                li  immTemp, imm
> +                xor dest, dest, immTemp
> +            */
> +            move(imm, immTempRegister);
> +            m_assembler.xorInsn(dest, dest, immTempRegister);
> +        }

Let's reorganize this code as follows,

if (!m_fixedWidth) {
    if (imm.m_value == -1) {
        ...
        return;
    }

    if (imm.m_value >= 0 && imm.m_value <= 65535) {
        ...;
        return;
    }
}
...;

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:704
> +        if (imm.m_value == -1 && !m_fixedWidth) {
>              m_assembler.nor(dest, src, MIPSRegisters::zero);
>              return;
>          }
> -
> -        /*
> -            li  immTemp, imm
> -            xor dest, dest, immTemp
> -        */
> -        move(imm, immTempRegister);
> -        m_assembler.xorInsn(dest, src, immTempRegister);
> +        if (imm.m_value >= 0 && imm.m_value <= 65535 && !m_fixedWidth)
> +            m_assembler.xori(dest, src, imm.m_value);
> +        else {
> +            /*
> +                li  immTemp, imm
> +                xor dest, src, immTemp
> +            */
> +            move(imm, immTempRegister);
> +            m_assembler.xorInsn(dest, src, immTempRegister);
> +        }

Ditto.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1628
> +        if (!m_fixedWidth && !right.m_value)
> +            return branch32(cond, left, MIPSRegisters::zero);
> +        if (right.m_value >= -32768 && right.m_value <= 32767 && !m_fixedWidth) {

Reorganize the function as follows,

if (!m_fixedWidth) {
    ...;
}
...default path...;

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1672
> +            if (cond == Equal) {
> +                move(right, immTempRegister);
> +                return branchEqual(left, immTempRegister);
> +            }
> +            if (cond == NotEqual) {
> +                move(right, immTempRegister);
> +                return branchNotEqual(left, immTempRegister);
> +            }
> +            if (cond == Above) {
> +                move(right, immTempRegister);
> +                m_assembler.sltu(cmpTempRegister, immTempRegister, left);
> +                return branchNotEqual(cmpTempRegister, MIPSRegisters::zero);
> +            }
> +            if (cond == AboveOrEqual) {
> +                m_assembler.sltiu(cmpTempRegister, left, right.m_value);
> +                return branchEqual(cmpTempRegister, MIPSRegisters::zero);
> +            }
> +            if (cond == Below) {
> +                m_assembler.sltiu(cmpTempRegister, left, right.m_value);
> +                return branchNotEqual(cmpTempRegister, MIPSRegisters::zero);
> +            }
> +            if (cond == BelowOrEqual) {
> +                move(right, immTempRegister);
> +                m_assembler.sltu(cmpTempRegister, immTempRegister, left);
> +                return branchEqual(cmpTempRegister, MIPSRegisters::zero);
> +            }
> +            if (cond == GreaterThan) {
> +                move(right, immTempRegister);
> +                m_assembler.slt(cmpTempRegister, immTempRegister, left);
> +                return branchNotEqual(cmpTempRegister, MIPSRegisters::zero);
> +            }
> +            if (cond == GreaterThanOrEqual) {
> +                m_assembler.slti(cmpTempRegister, left, right.m_value);
> +                return branchEqual(cmpTempRegister, MIPSRegisters::zero);
> +            }
> +            if (cond == LessThan) {
> +                m_assembler.slti(cmpTempRegister, left, right.m_value);
> +                return branchNotEqual(cmpTempRegister, MIPSRegisters::zero);
> +            }
> +            if (cond == LessThanOrEqual) {
> +                move(right, immTempRegister);
> +                m_assembler.slt(cmpTempRegister, immTempRegister, left);
> +                return branchEqual(cmpTempRegister, MIPSRegisters::zero);
> +            }

Large part of this function is the same to the default code path

move(...);
return branch32(...);

Let's add only special handling here. (AboveOrEqual, Below, GreaterThanOrEqual, LessThan).

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1674
> +            ASSERT(0);
> +            return Jump();

Remove this.

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1768
> +        }

Reorganize the function as follows,

if (!m_fixedWidth) {
   ...;
}
... default path ...;

> Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h:1791
> +            mask8 = MacroAssemblerHelpers::mask8OnCondition(*this, cond, mask);

I think we should not do this here. We should create a helper function mask8OnTest for MacroAssemblerMIPS and use it.
For example, test8 should also use it ;)

-- 
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/20180322/7e302138/attachment-0001.html>