Bug 99260

Summary: DFG should be able to emit effectful structure checks
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, mark.lam, mhahnenberg, msaboff, oliver, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 99262, 99269    
Bug Blocks: 98606    
Attachments:
Description Flags
the patch oliver: review+

Description Filip Pizlo 2012-10-13 23:06:06 PDT
Due to array type inference we may find ourselves entering a loop with a different array mode (and hence structure) than what we leave with.  Typically the loop will have an idempotent effect on both array type and structure: whenever it runs it transitions, either directly or indirectly, the array to exactly one structure.  This can be optimized using an effectful structure check: if an object doesn't have the structure we wanted, we attempt to convert to that structure and only OSR exit if we cannot do so.  If such an effectful structure check is hoisted, it ought to allow for rapid up-casting from bottom arrays to int arrays, int arrays to double arrays, and so on.
Comment 1 Filip Pizlo 2012-10-13 23:13:42 PDT
It would also be great if an effectful structure check could speculate must-not-alias.  Ordinarily an effectful structure check would have to be assumed by the CFA to clobber all structures, since it may change the structure of some object and all other objects we have references to may be aliased to that object.

But we could speculate against this by having the effectful slow path of an effectful structure check check all variables that have been proven to have some structure, and verify that they are not equal to the object being effectfully checked.
Comment 2 Filip Pizlo 2012-10-14 14:03:20 PDT
*** Bug 99269 has been marked as a duplicate of this bug. ***
Comment 3 Filip Pizlo 2012-10-28 17:18:33 PDT
Created attachment 171146 [details]
the patch
Comment 4 WebKit Review Bot 2012-10-28 17:23:34 PDT
Attachment 171146 [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/bytecode/ArrayProfile.h:119:  The parameter name "operation" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Filip Pizlo 2012-10-28 19:49:30 PDT
(In reply to comment #4)
> Attachment 171146 [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/bytecode/ArrayProfile.h:119:  The parameter name "operation" adds no information, so it should be removed.  [readability/parameter_name] [5]
> Total errors found: 1 in 20 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Fixed.
Comment 6 Filip Pizlo 2012-10-28 19:54:47 PDT
I should note that I changed the bug title to reflect that I'm only introducing the notion of effectful structure checks, and not hoisting them yet.  This is because with the current implementation of https://bugs.webkit.org/show_bug.cgi?id=98606, it's unlikely that a loop that accesses an array directly will see multiple array types unless there are additional indirections.  This is because in the programs I've seen, it's unlikely that you'll have a loop that operates directly over an array without mutating it - and if it mutates it, the baseline JIT will already LUB the array to a single type and the array allocation profile will henceforth only allocate arrays at the LUBed type.  So, what we worry about is loops of the form:

for (things) ... = array[i]

where the array can have multiple types.  It turns out that this kind of loop is considerably less likely in real code than:

for (things) ... = o.array[i]

or:

for (things) ... = o[i][j]

in which case hoisting doesn't buy you much.  Hence, I'm not going to implement hoisting, yet.

However, for such loops, having the effectful array check be a structure check instead of an indexing type check is profitable - it saves on some arithmetic, a register, and a load.  Also, the one remaining load is dead except for a branch, which tends to be quite fast in most architectures.
Comment 7 Filip Pizlo 2012-10-28 21:02:35 PDT
Landed in http://trac.webkit.org/changeset/132759