<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - FTL is not working on Windows."
href="https://bugs.webkit.org/show_bug.cgi?id=145366#c63">Comment # 63</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - FTL is not working on Windows."
href="https://bugs.webkit.org/show_bug.cgi?id=145366">bug 145366</a>
from <span class="vcard"><a class="email" href="mailto:mark.lam@apple.com" title="Mark Lam <mark.lam@apple.com>"> <span class="fn">Mark Lam</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=263482&action=diff" name="attach_263482" title="Patch">attachment 263482</a> <a href="attachment.cgi?id=263482&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=263482&action=review">https://bugs.webkit.org/attachment.cgi?id=263482&action=review</a>
<span class="quote">> Source/JavaScriptCore/CMakeLists.txt:952
> + llvm/LLVMAPI.cpp</span >
Since this file is common to WIN32 and other ports, let's keep it in the common list above.
<span class="quote">> Source/JavaScriptCore/CMakeLists.txt:958
> + llvm/LLVMAPI.cpp</span >
Ditto.
<span class="quote">> 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</span >
Why is this added? I don't see it used anywhere in this patch.
<span class="quote">> 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</span >
Why is this added? I don't see it used anywhere in this patch.
<span class="quote">> 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;
> + }</span >
Why is this added? I don't see it used anywhere in this patch.
<span class="quote">> 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);
> + }</span >
Are these really needed since their only caller in this patch does not appear to be called by anyone.
<span class="quote">> 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;</span >
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?
<span class="quote">> 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");
> + }
> + }</span >
I think this code will clash with <a class="bz_bug_link
bz_status_REOPENED "
title="REOPENED - FTL should generate a unique OSR exit for each duplicated OSR exit stackmap intrinsic."
href="show_bug.cgi?id=149970">https://bugs.webkit.org/show_bug.cgi?id=149970</a> (which is fixing a bug in this area).</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>