Bug 70384 – Support CanvasPixelArray in the DFG

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Oliver Hunt]

Support CanvasPixelArray in the DFG

[Oliver Hunt]

Created attachment 111534 [details]
Patch

[WebKit Review Bot]

Attachment 111534 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/assembler/X86Assembler.h:1090:  movb_rm is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 111534 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:344
> +            filter = PredictByteArray;

That's wrong, since you didn't implement the equivalent case in SpeculativeJIT.  This case for Array is here because SpeculativeJIT::compare() may filter on arrays.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:1330
> +        if (at(node.child1()).prediction() == PredictByteArray) {

Call isByteArrayPrediction

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:1392
> +        if (at(node.child1()).prediction() & PredictByteArray) {

Does the abstract state do the same thing?

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:1462
> +        if (at(node.child1()).prediction() & PredictByteArray) {

Does the abstract state do the same thing?

> Source/JavaScriptCore/wtf/ByteArray.cpp:28
> +

kill!

[Oliver Hunt]

Created attachment 111544 [details]
Patch

[Geoffrey Garen]

Comment on attachment 111544 [details]
Patch

Marking r- based on Phil's comments.

[WebKit Review Bot]

Attachment 111544 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/assembler/X86Assembler.h:1090:  movb_rm is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 111544 [details]
Patch

Oops! I was behind. Unreviewing.

[Gavin Barraclough]

Comment on attachment 111544 [details]
Patch

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

> Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:533
> +        m_assembler.movb_rm(src, address.offset, address.base, address.index, address.scale);

I believe this is unsafe on 32-bit x86.

[Oliver Hunt]

Created attachment 111659 [details]
Patch

[Filip Pizlo]

Comment on attachment 111659 [details]
Patch

r=me

[Oliver Hunt]

Committed r97876: <http://trac.webkit.org/changeset/97876>