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)
commit78e87c74dea8bb54b94d2cc19c06f2ce7023b787
tree04f089dc34bc04eb7abc619c398a5a0ae049cbf9
parent81ef511a118f2b7c63fa32ec6d9a2f563f8aceab
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: http://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