Bug 134990 - Need FTL implementation of SkipTopScope
Summary: Need FTL implementation of SkipTopScope
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Matthew Mirman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-16 14:23 PDT by Matthew Mirman
Modified: 2014-07-29 14:52 PDT (History)
4 users (show)

See Also:

Attachments
[ftlopt] Added SkipTopScope coverage to FTL (3.11 KB, patch)
2014-07-16 14:28 PDT, Matthew Mirman 		ggaren: review+
ggaren: commit-queue-
Details | Formatted Diff | Diff
[ftlopt] Added SkipTopScope coverage to FTL (3.08 KB, patch)
2014-07-16 15:00 PDT, Matthew Mirman 		fpizlo: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Mirman 2014-07-16 14:23:55 PDT 
patch forthcoming.
Comment 1 Matthew Mirman 2014-07-16 14:28:17 PDT 
Created attachment 235021 [details]
[ftlopt] Added SkipTopScope coverage to FTL

Can't test it yet though.
Comment 2 WebKit Commit Bot 2014-07-16 14:29:59 PDT 
Attachment 235021 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:3394:  Extra space after ( in function call  [whitespace/parens] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:3404:  Missing space after ,  [whitespace/comma] [3]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Geoffrey Garen 2014-07-16 14:37:45 PDT 
Comment on attachment 235021 [details]
[ftlopt] Added SkipTopScope coverage to FTL

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

r=me

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:3421
> +            m_out.isZero64( m_out.load64(addressFor(machineActivationRegister)))

No space after the paren here.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:3423
> +            , unsure(isNull)
> +            , unsure(notNull));

These two can probably go on the same line.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:3431
> +        setJSValue(m_out.phi(m_out.intPtr, top,next));

Space after "top".
Comment 4 Matthew Mirman 2014-07-16 15:00:17 PDT 
Created attachment 235024 [details]
[ftlopt] Added SkipTopScope coverage to FTL

Fixed style
Comment 5 Matthew Mirman 2014-07-29 14:52:58 PDT 
Landed in http://trac.webkit.org/changeset/171767