Bug 111102

Summary: It should be easy to determine if a DFG node exits forward or backward when doing type checks
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, mario, mark.lam, mhahnenberg, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 109389    
Attachments:
Description Flags
the patch mhahnenberg: review+

Filip Pizlo
Reported 2013-02-28 12:38:16 PST
Right now, some nodes type check with forward exit, while others do it with backward exit. Most nodes will use the same direction for all of their exits (the one exception is some typed array access). But the only way you can even get a hint of exit directionality is by casing on node type. This is sloppy, and could lead to bugs (if you replace a ForwardCheckStructure with a Phantom and intend to keep the cell check, you've now introduced a really bad bug). This patch fixes this by having a NodeFlag that tells the exit directionality.
Attachments
the patch (57.01 KB, patch)
2013-02-28 12:44 PST, Filip Pizlo 		mhahnenberg: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2013-02-28 12:44:44 PST
Created attachment 190785 [details] the patch
Mark Hahnenberg
Comment 2 2013-02-28 12:58:51 PST
Comment on attachment 190785 [details] the patch r=me
Oliver Hunt
Comment 3 2013-02-28 13:00:15 PST
Comment on attachment 190785 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=190785&action=review > Source/JavaScriptCore/dfg/DFGNodeFlags.h:70 > +#define NodeExitsForward 0x10000 enum { NodeExitsForward = 0x10000 }? I've been leaning towards enums for constants lately as they're guaranteed to be constant (eg. no compiler deciding to do stupid loads) and are visible to the debugger
Filip Pizlo
Comment 4 2013-02-28 13:05:43 PST
(In reply to comment #3) > (From update of attachment 190785 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=190785&action=review > > > Source/JavaScriptCore/dfg/DFGNodeFlags.h:70 > > +#define NodeExitsForward 0x10000 > > enum { NodeExitsForward = 0x10000 }? > > I've been leaning towards enums for constants lately as they're guaranteed to be constant (eg. no compiler deciding to do stupid loads) and are visible to the debugger Separate patch? DFGNodeFlags.h uses #define's extensively for all of the flags, and has been doing so since the DFG's beginning.
Filip Pizlo
Comment 5 2013-02-28 13:54:18 PST
Landed in http://trac.webkit.org/changeset/144362
Note You need to log in before you can comment on or make changes to this bug.