Bug 134015 – add ftl infrastructure to windows

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Alex Christensen]

This is part of why I want the win64 jit so bad.  I got the jit and dfg working with https://bugs.webkit.org/show_bug.cgi?id=130638 and I just got the ftl to compile and link.  libllvmForJSC compiles and mostly links.

[Alex Christensen]

Created attachment 233288 [details]
Patch

[Build Bot]

Comment on attachment 233288 [details]
Patch

Attachment 233288 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5327732833517568

New failing tests:
media/W3C/video/readyState/readyState_during_canplay.html

[Build Bot]

Created attachment 233298 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Alex Christensen]

The only possible behavior change on mac would be if std::nan("") is not the same as 0.0 / 0.0.

[Filip Pizlo]

Comment on attachment 233288 [details]
Patch

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

> Source/JavaScriptCore/disassembler/LLVMDisassembler.cpp:57
> +        _snprintf(

Can we have one common #define or something in WTF so that we don't have to #if PLATFORM in so many places just for this function?

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1439
> +                        right, m_out.constDouble(std::nan("")))));

We should use PNaN here. It's a JSC thing.

[Alex Christensen]

Created attachment 233336 [details]
Patch

[Alex Christensen]

(In reply to comment #5)
> (From update of attachment 233288 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=233288&action=review
> 
> > Source/JavaScriptCore/disassembler/LLVMDisassembler.cpp:57
> > +        _snprintf(
> 
> Can we have one common #define or something in WTF so that we don't have to #if PLATFORM in so many places just for this function?
I'll just leave that out of this patch until I can get a chance to test it.  There's also an issue with _snprintf not always writing a null character at the end if the string is the exact length of the buffer.

> 
> > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1439
> > +                        right, m_out.constDouble(std::nan("")))));
> 
> We should use PNaN here. It's a JSC thing.
Done.

[Filip Pizlo]

Comment on attachment 233336 [details]
Patch

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

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1439
> -                right, m_out.constDouble(0.0 / 0.0))));
> +                        right, m_out.constDouble(PNaN))));

I don't think it's right to change the indentation here.

[Alex Christensen]

(In reply to comment #8)
> I don't think it's right to change the indentation here.
Style bot does, but I didn't change it in the commit.

http://trac.webkit.org/changeset/170130