Bug 70384

Summary: Support CanvasPixelArray in the DFG
Product: WebKit Reporter: Oliver Hunt <oliver>
Component: New BugsAssignee: Oliver Hunt <oliver>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch fpizlo: review+

Oliver Hunt
Reported 2011-10-18 16:48:08 PDT
Support CanvasPixelArray in the DFG
Attachments
Patch (34.92 KB, patch)
2011-10-18 17:17 PDT, Oliver Hunt
no flags
Patch (33.97 KB, patch)
2011-10-18 18:18 PDT, Oliver Hunt
no flags
Patch (34.52 KB, patch)
2011-10-19 12:41 PDT, Oliver Hunt
fpizlo: review+
Oliver Hunt
Comment 1 2011-10-18 17:17:02 PDT
WebKit Review Bot
Comment 2 2011-10-18 17:19:49 PDT
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 3 2011-10-18 17:28:21 PDT
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
Comment 4 2011-10-18 18:18:04 PDT
Geoffrey Garen
Comment 5 2011-10-18 18:19:18 PDT
Comment on attachment 111544 [details] Patch Marking r- based on Phil's comments.
WebKit Review Bot
Comment 6 2011-10-18 18:20:11 PDT
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 7 2011-10-18 18:20:52 PDT
Comment on attachment 111544 [details] Patch Oops! I was behind. Unreviewing.
Gavin Barraclough
Comment 8 2011-10-19 12:22:37 PDT
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
Comment 9 2011-10-19 12:41:58 PDT
Filip Pizlo
Comment 10 2011-10-19 14:05:00 PDT
Comment on attachment 111659 [details] Patch r=me
Oliver Hunt
Comment 11 2011-10-19 14:24:23 PDT
Note You need to log in before you can comment on or make changes to this bug.