Bug 127589

Summary: GetTypedArrayOffset in FTL
Product: WebKit Reporter: Matthew Mirman <mmirman>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
GetTypedArrayOffset in FTL implemented
fpizlo: review-, fpizlo: commit-queue-
GetTypedArrayOffset in FTL implemented
none
GetTypedArrayOffset in FTL implemented
none
GetTypedArrayOffset in FTL implemented none

Description Matthew Mirman 2014-01-24 14:54:35 PST
patch forthcoming
Comment 1 Matthew Mirman 2014-01-24 14:58:22 PST
Created attachment 222159 [details]
GetTypedArrayOffset in FTL implemented

added two test cases for this.
Comment 2 WebKit Commit Bot 2014-01-24 14:59:42 PST
Attachment 222159 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-wasteful.js:2:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-wasteful.js:8:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-wasteful.js:10:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1472:  Extra space after ( in function call  [whitespace/parens] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1473:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-simple.js:2:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-simple.js:8:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-simple.js:10:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 8 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Filip Pizlo 2014-01-24 15:01:07 PST
Comment on attachment 222159 [details]
GetTypedArrayOffset in FTL implemented

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

Almost there but fix style.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1629
> +        m_out.branch( m_out.notEqual( baseAddress , m_out.constIntPtr(WastefulTypedArray))
> +                    , simpleCase
> +                    , wastefulCase
> +                    );

Bad style:

- Shouldn't have a space between "branch(" and "m_out", and then again between "notEqual(" and "baseAddress".  In general we don't do spaces between the open paren and the stuff inside.

- Should put the commas on the previous line at those linebreaks.

- When you wrap a line, always indent 4 spaces and never more.

> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:1651
> +        // output

You probably don't need this comment.
Comment 4 Matthew Mirman 2014-01-24 17:02:55 PST
Created attachment 222168 [details]
GetTypedArrayOffset in FTL implemented

fixed whitespace in last version.
Comment 5 WebKit Commit Bot 2014-01-24 17:04:21 PST
Attachment 222168 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-wasteful.js:2:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-wasteful.js:8:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-wasteful.js:10:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-simple.js:2:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-simple.js:8:  Line contains tab character.  [whitespace/tab] [5]
ERROR: Source/JavaScriptCore/tests/stress/ftl-gettypedarrayoffset-simple.js:10:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 6 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Matthew Mirman 2014-01-24 17:04:37 PST
Created attachment 222169 [details]
GetTypedArrayOffset in FTL implemented

Fixed even more whitespace
Comment 7 Matthew Mirman 2014-01-24 18:00:48 PST
Created attachment 222178 [details]
GetTypedArrayOffset in FTL implemented

Even more whitespace fixes.
Comment 8 Geoffrey Garen 2014-01-27 11:39:52 PST
FYI, you should be able to emacs-mode your way out of future whitespace problems.
Comment 9 WebKit Commit Bot 2014-02-03 13:47:00 PST
Comment on attachment 222178 [details]
GetTypedArrayOffset in FTL implemented

Clearing flags on attachment: 222178

Committed r163324: <http://trac.webkit.org/changeset/163324>
Comment 10 WebKit Commit Bot 2014-02-03 13:47:02 PST
All reviewed patches have been landed.  Closing bug.