RESOLVED FIXED 93150
Array checks should use the structure, not the class info 
https://bugs.webkit.org/show_bug.cgi?id=93150
Summary Array checks should use the structure, not the class info
Filip Pizlo
Reported 2012-08-03 12:18:43 PDT
This implies that the LLInt and baseline JIT must profile which structure was observed, so that the DFG may pick the right structure to check on the fast path.
Attachments
work in progress (26.95 KB, patch)
2012-08-03 12:22 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
more (33.86 KB, patch)
2012-08-03 18:07 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (71.52 KB, patch)
2012-08-13 23:57 PDT, Filip Pizlo 		mhahnenberg: review+
buildbot: commit-queue-
DetailsFormatted DiffDiff
patch for landing, hopefully (77.51 KB, patch)
2012-08-14 18:57 PDT, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2012-08-03 12:22:58 PDT
Created attachment 156431 [details] work in progress
Filip Pizlo
Comment 2 2012-08-03 18:07:32 PDT
Created attachment 156494 [details] more
Filip Pizlo
Comment 3 2012-08-13 23:57:06 PDT
Created attachment 158235 [details] the patch
WebKit Review Bot
Comment 4 2012-08-14 00:01:39 PDT
Attachment 158235 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/bytecode/Instruction.h:49: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/bytecode/ArrayProfile.h:79: The parameter name "operation" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 5 2012-08-14 01:22:36 PDT
Comment on attachment 158235 [details] the patch Attachment 158235 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13501104
Mark Hahnenberg
Comment 6 2012-08-14 14:32:01 PDT
Comment on attachment 158235 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=158235&action=review Looks good overall except for minor fix. Also placate windows and style bot please :-) > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:214 > + addSlowCase(branchPtr(NotEqual, Address(regT1, JSCell::classInfoOffset()), TrustedImmPtr(&JSArray::s_info))); Should this be Structure::classInfoOffset()?
Filip Pizlo
Comment 7 2012-08-14 16:00:05 PDT
(In reply to comment #6) > (From update of attachment 158235 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158235&action=review > > Looks good overall except for minor fix. Also placate windows and style bot please :-) > > > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:214 > > + addSlowCase(branchPtr(NotEqual, Address(regT1, JSCell::classInfoOffset()), TrustedImmPtr(&JSArray::s_info))); > > Should this be Structure::classInfoOffset()? Good catch! Thanks!
Filip Pizlo
Comment 8 2012-08-14 18:57:34 PDT
Created attachment 158475 [details] patch for landing, hopefully
WebKit Review Bot
Comment 9 2012-08-14 19:00:39 PDT
Attachment 158475 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/bytecode/Instruction.h:49: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/bytecode/ArrayProfile.h:79: The parameter name "operation" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 32 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filip Pizlo
Comment 10 2012-08-14 19:49:26 PDT
Landed in http://trac.webkit.org/changeset/125637
Note You need to log in before you can comment on or make changes to this bug.