Bug 78515 - DFG should be able to emit code on control flow edges
Summary: DFG should be able to emit code on control flow edges
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2012-02-13 12:00 PST by Filip Pizlo
Modified: 2012-02-16 02:11 PST (History)
2 users (show)

See Also:

the patch (40.76 KB, patch)
2012-02-13 12:15 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (40.82 KB, patch)
2012-02-13 13:26 PST, Filip Pizlo
barraclough: review+
webkit-ews: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2012-02-13 12:00:49 PST
Currently the DFG can emit code for the basic blocks that appeared in the program.  But it is often necessary or useful to emit code on synthetic basic blocks created on control flow edges.  For example we may have B1, B2, B3 such that B1 branches to B2 or some other block, and B3 jumps to B2.  We may want some code to execute only when B1 branches to B2 but not when it branches elsewhere; and for a different piece of code to execute when B3 jumps to B2.

Currently the code that emits branches makes this difficult, since if it directly emits branches, and there is no way to say that some code should be inserted just on one of the edges of the branch.

To make this work, we should add the notion of control flow edge code, and abstract this behind an API for emitting branches so that most of the code generator doesn't have to worry about it.  Since currently we do not yet use this feature, we should have a verification mode that emits dummy code in control flow edges to test that this feature is working correctly.
Comment 1 Filip Pizlo 2012-02-13 12:15:58 PST
Created attachment 126803 [details]
the patch

Note - because I was lazy this patch has verification enabled, since I'm still testing it.  I will land with DFG_ENABLE_EDGE_CODE_VERIFICATION set to 0.
Comment 2 Filip Pizlo 2012-02-13 13:26:06 PST
Created attachment 126816 [details]
the patch

Fixed Mac build, turned off landing pads.
Comment 3 Early Warning System Bot 2012-02-13 14:22:30 PST
Comment on attachment 126816 [details]
the patch

Attachment 126816 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11511601
Comment 4 Gavin Barraclough 2012-02-13 15:54:24 PST
Comment on attachment 126816 [details]
the patch

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

> Source/JavaScriptCore/assembler/MacroAssembler.h:78
> +    static DoubleCondition invert(DoubleCondition cond)

At some point we should probably make double condition a common type for all assemblers (independent of machine implementation), with a guarantee that we can just invert by flipping a bit, and sue a table to map from the abstract type to a machine type.  Ditto for the integer conditions.  This looks fine for now though.

> Source/JavaScriptCore/assembler/MacroAssembler.h:110
> +    static bool isInvertible(ResultCondition cond)

Presumably we could do away with this by introducing NoOverflow/NotSigned types? Probably worth doing at some point, though not urgent.