[Webkit-unassigned] [Bug 145366] FTL is not working on Windows.

[bugzilla-daemon at webkit.org]

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

--- Comment #63 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 263482
  --> https://bugs.webkit.org/attachment.cgi?id=263482
Patch

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

> Source/JavaScriptCore/CMakeLists.txt:952
> +            llvm/LLVMAPI.cpp

Since this file is common to WIN32 and other ports, let's keep it in the common list above.

> Source/JavaScriptCore/CMakeLists.txt:958
> +            llvm/LLVMAPI.cpp

Ditto.

> Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:722
> +#if CPU(X86_64)
> +    void loadDouble128(ImplicitAddress address, FPRegisterID src)
> +    {
> +        ASSERT(isSSE2Present());
> +        m_assembler.movups_mr(src, address.offset, address.base);
> +    }
> +#endif

Why is this added?  I don't see it used anywhere in this patch.

> Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:747
> +#if CPU(X86_64)
> +    void storeDouble128(FPRegisterID src, ImplicitAddress address)
> +    {
> +        ASSERT(isSSE2Present());
> +        m_assembler.movups_rm(src, address.offset, address.base);
> +    }
> +#endif

Why is this added?  I don't see it used anywhere in this patch.

> Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:197
> +    Call callJIT()
> +    {
> +        DataLabelPtr label = moveWithPatch(TrustedImmPtr(0), scratchRegister);
> +        Call result = Call(m_assembler.call(scratchRegister), Call::Linkable);
> +
> +        ASSERT_UNUSED(label, differenceBetween(label, result) == REPTACH_OFFSET_CALL_R11);
> +        return result;
> +    }

Why is this added?  I don't see it used anywhere in this patch.

> Source/JavaScriptCore/assembler/X86Assembler.h:1848
> +    void movups_rm(XMMRegisterID src, int offset, RegisterID base)
> +    {
> +        m_formatter.twoByteOp(OP2_MOVSD_WsdVsd, (RegisterID)src, base, offset);
> +    }
> +
> +    void movups_mr(XMMRegisterID src, int offset, RegisterID base)
> +    {
> +        m_formatter.twoByteOp(OP2_MOVSD_VsdWsd, (RegisterID)src, base, offset);
> +    }

Are these really needed since their only caller in this patch does not appear to be called by anyone.

> Source/JavaScriptCore/ftl/FTLCapabilities.cpp:213
> +    case ArithPow:
> +#if OS(WINDOWS)
> +        // LLVM does not support powi on Windows.
> +        if (node->child2().useKind() == Int32Use)
> +            return CannotCompile;
> +#endif
> +        break;

Is this still necessary?  In compileArithPow() below, you seem to have implemented support for (m_node->child2().useKind() == Int32Use) already (line 1843).  Or am I misreading it?

> Source/JavaScriptCore/ftl/FTLCompile.cpp:680
> +            auto iter = recordMap.find(call.stackmapID());
> +            if (iter != recordMap.end()) {
> +                for (unsigned i = 0; i < iter->value.size(); ++i) {
> +                    StackMaps::Record& record = iter->value[i];
> +                    RegisterSet usedRegisters = usedRegistersFor(record);
> +
> +                    dataLog("Used registers = ", usedRegisters, "\n");
> +                }
> +            }

I think this code will clash with https://bugs.webkit.org/show_bug.cgi?id=149970 (which is fixing a bug in this area).

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151019/8a2a2126/attachment-0001.html>