Bug 177806 - [FTL] Add feature to delay materialization when OSR exit occurs if node is only used by MovHint
Summary: [FTL] Add feature to delay materialization when OSR exit occurs if node is on...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-03 02:48 PDT by Yusuke Suzuki
Modified: 2017-10-03 17:30 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2017-10-03 02:48:29 PDT
We would like to extend Object Allocation Sinking (OAS) phase to the other operations that does not have side effect.
By doing so, we can eliminate these operations even if it is pointed by object properties or MovHint.

The thing in my mind is UInt32ToNumber, which can remain due to MovHint. But it can be removed if we leverage escape analysis done in OAS, by introducing PhantomUInt32ToNumber node.
So, we would like to extend OAS to handle nodes other than allocations.

I would like to introduce this change separately from bug 177281 since bug 177281 is a bit large and it already handles things seen in emscripten well.
Comment 1 Yusuke Suzuki 2017-10-03 03:16:23 PDT
(In reply to Yusuke Suzuki from comment #0)
> We would like to extend Object Allocation Sinking (OAS) phase to the other
> operations that does not have side effect.
> By doing so, we can eliminate these operations even if it is pointed by
> object properties or MovHint.
> 
> The thing in my mind is UInt32ToNumber, which can remain due to MovHint. But
> it can be removed if we leverage escape analysis done in OAS, by introducing
> PhantomUInt32ToNumber node.
> So, we would like to extend OAS to handle nodes other than allocations.
> 
> I would like to introduce this change separately from bug 177281 since bug
> 177281 is a bit large and it already handles things seen in emscripten well.

Another target is MakeRope.
Comment 2 Yusuke Suzuki 2017-10-03 03:17:35 PDT
(In reply to Yusuke Suzuki from comment #1)
> (In reply to Yusuke Suzuki from comment #0)
> > We would like to extend Object Allocation Sinking (OAS) phase to the other
> > operations that does not have side effect.
> > By doing so, we can eliminate these operations even if it is pointed by
> > object properties or MovHint.
> > 
> > The thing in my mind is UInt32ToNumber, which can remain due to MovHint. But
> > it can be removed if we leverage escape analysis done in OAS, by introducing
> > PhantomUInt32ToNumber node.
> > So, we would like to extend OAS to handle nodes other than allocations.
> > 
> > I would like to introduce this change separately from bug 177281 since bug
> > 177281 is a bit large and it already handles things seen in emscripten well.
> 
> Another target is MakeRope.

And this reduces the burden of removing MovHint aggressively. Even in the face of conservative / sub-optimal MovHint elimination, we can still remove bunch of unnecessary materializations.
Comment 3 Filip Pizlo 2017-10-03 06:07:07 PDT
That phase has so much complexity because it is escape-analyzing. You don’t have to escape analyze anything to use materialization to side-step MovHints during DCE. So, whatever you do, it should probably not be part of that phase.
Comment 4 Yusuke Suzuki 2017-10-03 07:36:01 PDT
(In reply to Filip Pizlo from comment #3)
> That phase has so much complexity because it is escape-analyzing. You don’t
> have to escape analyze anything to use materialization to side-step MovHints
> during DCE. So, whatever you do, it should probably not be part of that
> phase.

Is it better to have a new phase for this optimization?
Comment 5 Filip Pizlo 2017-10-03 08:34:40 PDT
(In reply to Yusuke Suzuki from comment #4)
> (In reply to Filip Pizlo from comment #3)
> > That phase has so much complexity because it is escape-analyzing. You don’t
> > have to escape analyze anything to use materialization to side-step MovHints
> > during DCE. So, whatever you do, it should probably not be part of that
> > phase.
> 
> Is it better to have a new phase for this optimization?

Yeah!  Go ahead and add a new phase!

Another reason to add a new phase: ObjectAllocationSinking is the single most expensive phase, so we probably want to rethink some of its data structures. If it was also tracking pure operations, it would be even more expensive. 

But fundamentally: object allocation sinking is running a must-points-to analysis. You don’t need that.
Comment 6 Yusuke Suzuki 2017-10-03 17:30:11 PDT
(In reply to Filip Pizlo from comment #5)
> (In reply to Yusuke Suzuki from comment #4)
> > (In reply to Filip Pizlo from comment #3)
> > > That phase has so much complexity because it is escape-analyzing. You don’t
> > > have to escape analyze anything to use materialization to side-step MovHints
> > > during DCE. So, whatever you do, it should probably not be part of that
> > > phase.
> > 
> > Is it better to have a new phase for this optimization?
> 
> Yeah!  Go ahead and add a new phase!
> 
> Another reason to add a new phase: ObjectAllocationSinking is the single
> most expensive phase, so we probably want to rethink some of its data
> structures. If it was also tracking pure operations, it would be even more
> expensive. 
> 
> But fundamentally: object allocation sinking is running a must-points-to
> analysis. You don’t need that.

OK, sounds reasonable! I'll create a new phase for that :)