Bug 123788 - GetById->GetByOffset and PutById->PutByOffset folding should mark haveStructures since it may result in structure transition watchpoints
Summary: GetById->GetByOffset and PutById->PutByOffset folding should mark haveStructu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-04 22:37 PST by Filip Pizlo
Modified: 2013-11-05 10:51 PST (History)
7 users (show)

See Also:

Attachments
the patch (2.26 KB, patch)
2013-11-04 22:44 PST, Filip Pizlo 		ggaren: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-11-04 22:37:14 PST 
This is totally asymptomatic as far as I can tell even though it's a violation of IR semantics.  It just sort of "works" because this only arises if you already had a transition watchpoint for that structure, which usually ends up being good enough since watchpoints invalidate the whole function.
Comment 1 Filip Pizlo 2013-11-04 22:44:03 PST 
Created attachment 216005 [details]
the patch
Comment 2 Geoffrey Garen 2013-11-05 10:37:00 PST 
Comment on attachment 216005 [details]
the patch

r=me

The phrase "have structures" does not communicate this detail to me. I think by "have" you mean "speculated about, in a way which is invalid after an operation that clobbers the world". Maybe a better name would be "hasStructureSpeculations" or "hasStructuresToClobber" or "hasClobberableStructures".
Comment 3 Filip Pizlo 2013-11-05 10:45:47 PST 
(In reply to comment #2)
> (From update of attachment 216005 [details])
> r=me
> 
> The phrase "have structures" does not communicate this detail to me. I think by "have" you mean "speculated about, in a way which is invalid after an operation that clobbers the world". Maybe a better name would be "hasStructureSpeculations" or "hasStructuresToClobber" or "hasClobberableStructures".

Fair point.  I'll try to think of a better name.
Comment 4 Filip Pizlo 2013-11-05 10:51:17 PST 
Landed in http://trac.webkit.org/changeset/158680