Object allocation sinking phase doesn't properly handle control flow when emitting...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 11 Feb 2017 04:05:06 +0000 (04:05 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 11 Feb 2017 04:05:06 +0000 (04:05 +0000)
commitba175c205073c0d8ca2211eeb4220ba6bf781dd6
tree04f089dc34bc04eb7abc619c398a5a0ae049cbf9
parent117ec874b2b7e9ed30336adcece70143eec6e78e
Object allocation sinking phase doesn't properly handle control flow when emitting a PutHint of a materialized object into a PromotedHeapLocation of a still sunken object
https://bugs.webkit.org/show_bug.cgi?id=168140
<rdar://problem/30205880>

Reviewed by Filip Pizlo.

JSTests:

* stress/allocation-sinking-puthint-control-flow.js: Added.
(e):
(bar):
(let.y):
(else.let.y):
(baz):
(foo):
(catch):

Source/JavaScriptCore:

This patch fixes a bug in allocation sinking phase where
we don't properly handle control flow when materializing
an object and also PutHinting that materialization into
a still sunken object. We were performing the PutHint
for the materialization at the point of materialization,
however, we may have materialized along both edges
of a control flow diamond, in which case, we need to
also PutHint at the join point. Consider this program:

```
bb#0:
b: PhantomActivation()
a: PhantomNewFunction()
c: PutHint(@a, @b, ActivationLoc)
Branch(#1, #2)

bb#1:
d: MaterializeActivation()
e: PutHint(@a, @d, ActivationLoc)
f: Upsilon(@d, ^p)
Jump(#3)

bb#2:
g: MaterializeActivation()
h: PutHint(@a, @g, ActivationLoc)
i: Upsilon(@d, ^p)
Jump(#3)

bb#3:
p: Phi()
// What is PromotedHeapLocation(@a, ActivationLoc) here?
// What would we do if we exited?
```
Before this patch, we didn't perform a PutHint of the Phi.
However, we need to, otherwise when exit, we won't know
the value of PromotedHeapLocation(@a, ActivationLoc)

The program we need then, for correctness, is this:
```
bb#0:
b: PhantomActivation()
a: PhantomNewFunction()
c: PutHint(@a, @b, ActivationLoc)
Branch(#1, #2)

bb#1:
d: MaterializeActivation()
e: PutHint(@a, @d, ActivationLoc)
f: Upsilon(@d, ^p)
Jump(#3)

bb#2:
g: MaterializeActivation()
h: PutHint(@a, @g, ActivationLoc)
i: Upsilon(@d, ^p)
Jump(#3)

bb#3:
p: Phi()
j: PutHint(@a, @p, ActivationLoc)
```

This patch makes it so that we emit the necessary PutHint at node `j`.
I've also added more validation to the OSRAvailabilityAnalysisPhase
to catch this problem during validation.

* dfg/DFGOSRAvailabilityAnalysisPhase.cpp:
(JSC::DFG::OSRAvailabilityAnalysisPhase::run):
* dfg/DFGObjectAllocationSinkingPhase.cpp:
* ftl/FTLOperations.cpp:
(JSC::FTL::operationMaterializeObjectInOSR):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@212177 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JSTests/ChangeLog
JSTests/stress/allocation-sinking-puthint-control-flow.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp
Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp
Source/JavaScriptCore/ftl/FTLOperations.cpp