Bug 188707 – intersectionOfPastValuesAtHead must filter values after they've observed an invalidation point

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

...

[Saam Barati]

<rdar://problem/43015442>

[Saam Barati]

Created attachment 347392 [details]
patch

[Mark Lam]

Comment on attachment 347392 [details]
patch

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

r=me

> Source/JavaScriptCore/ChangeLog:29
> +        We fix this by filtering values in intersectionOfPastValuesAtHead as
> +        with values as if an invalidation point has occured. This places the

The text "intersectionOfPastValuesAtHead as with values as if" is unclear.  Looks like there's some partial edits here.  Please fix.

[Saam Barati]

Created attachment 347398 [details]
patch for landing

[Geoffrey Garen]

Comment on attachment 347398 [details]
patch for landing

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

> Source/JavaScriptCore/dfg/DFGCFAPhase.cpp:147
> +                    // the incoming value as if an invalidation point has occurred.

as if it could live past an invalidation point

> Source/JavaScriptCore/dfg/DFGCFAPhase.cpp:149
> +                    // structure, and an InvalidationPoint no longer guarantees

It’s more like we’re violating the promise made by an invalidation point. The invalidating point itself still works.  We must conservatively act as if there’s an invalidation point because we do not pr Isley model them in osr entry metadata.

[Saam Barati]

Created attachment 347412 [details]
patch for landing

[Saam Barati]

Created attachment 347414 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 347414 [details]
patch for landing

Rejecting attachment 347414 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 347414, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in JSTests/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/8897685

[Saam Barati]

landed in:
https://trac.webkit.org/changeset/235007/webkit

[Filip Pizlo]

(In reply to Geoffrey Garen from comment #5)
> Comment on attachment 347398 [details]
> patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347398&action=review
> 
> > Source/JavaScriptCore/dfg/DFGCFAPhase.cpp:147
> > +                    // the incoming value as if an invalidation point has occurred.
> 
> as if it could live past an invalidation point
> 
> > Source/JavaScriptCore/dfg/DFGCFAPhase.cpp:149
> > +                    // structure, and an InvalidationPoint no longer guarantees
> 
> It’s more like we’re violating the promise made by an invalidation point.
> The invalidating point itself still works.  We must conservatively act as if
> there’s an invalidation point because we do not pr Isley model them in osr
> entry metadata.

OSR happens at a point in time where if any watchpoints fired, we would have jettisoned the code block. Therefore, the most precise modeling of invalidation points at OSR is to say that they had happened. So, we are modeling them precisely in this patch.