WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
144166
Function allocation sinking shouldn't be performed on singleton functions
https://bugs.webkit.org/show_bug.cgi?id=144166
Summary
Function allocation sinking shouldn't be performed on singleton functions
Basile Clement
Reported
2015-04-24 15:42:55 PDT
Patch forthcoming.
Attachments
Patch
(2.86 KB, patch)
2015-04-24 16:21 PDT
,
Basile Clement
ggaren
: review+
ggaren
: commit-queue-
Details
Formatted Diff
Diff
Fix ChangeLog typos
(2.86 KB, patch)
2015-04-24 16:25 PDT
,
Basile Clement
fpizlo
: review+
Details
Formatted Diff
Diff
Simplify ChangeLog entry
(2.80 KB, patch)
2015-04-27 12:54 PDT
,
Basile Clement
no flags
Details
Formatted Diff
Diff
Patch for landing
(2.96 KB, patch)
2015-05-01 14:46 PDT
,
Basile Clement
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Basile Clement
Comment 1
2015-04-24 16:21:11 PDT
Created
attachment 251586
[details]
Patch
Geoffrey Garen
Comment 2
2015-04-24 16:23:31 PDT
Comment on
attachment 251586
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=251586&action=review
r=me cq- for some ChangeLog edits.
> Source/JavaScriptCore/ChangeLog:8 > + Function allocation usually are free of any other side effects, but
allocation => allocations
> Source/JavaScriptCore/ChangeLog:18 > + to worry about its watchpoint, allowing to use
allowing to use => allowing us to use
Basile Clement
Comment 3
2015-04-24 16:25:22 PDT
Created
attachment 251587
[details]
Fix ChangeLog typos
Basile Clement
Comment 4
2015-04-24 16:26:35 PDT
(In reply to
comment #2
)
> Comment on
attachment 251586
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=251586&action=review
> > r=me
Thanks!
> > cq- for some ChangeLog edits. > > > Source/JavaScriptCore/ChangeLog:8 > > + Function allocation usually are free of any other side effects, but > > allocation => allocations > > > Source/JavaScriptCore/ChangeLog:18 > > + to worry about its watchpoint, allowing to use > > allowing to use => allowing us to use
Fixed those in a new patch.
Filip Pizlo
Comment 5
2015-04-27 12:28:42 PDT
Comment on
attachment 251587
[details]
Fix ChangeLog typos View in context:
https://bugs.webkit.org/attachment.cgi?id=251587&action=review
Holding off cq because the ChangeLog comment could be refined.
> Source/JavaScriptCore/ChangeLog:14 > + watchpoints invalidating code that depends on it being a singleton). According > + to fpizlo, this happens to be sinkable right now, but things may not stay that > + way in the future (e.g. it would not be the case for CreateActivation), and > + fixing this now would help prevent hard-to-find bugs later in that case.
What won't stay that way in the future? That singleton function allocations are sinkable? I think it's enough to say that we don't want to sink code that has effects if we haven't done the work to prove that the effect wasn't observed. Object allocation sinking phase does no such analysis because it assumes that the act of allocating is not effectful.
Basile Clement
Comment 6
2015-04-27 12:41:45 PDT
(In reply to
comment #5
)
> Comment on
attachment 251587
[details]
> Fix ChangeLog typos > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=251587&action=review
> > Holding off cq because the ChangeLog comment could be refined. > > > Source/JavaScriptCore/ChangeLog:14 > > + watchpoints invalidating code that depends on it being a singleton). According > > + to fpizlo, this happens to be sinkable right now, but things may not stay that > > + way in the future (e.g. it would not be the case for CreateActivation), and > > + fixing this now would help prevent hard-to-find bugs later in that case. > > What won't stay that way in the future? That singleton function allocations > are sinkable? > > I think it's enough to say that we don't want to sink code that has effects > if we haven't done the work to prove that the effect wasn't observed. > Object allocation sinking phase does no such analysis because it assumes > that the act of allocating is not effectful.
OK, will simplify that.
Basile Clement
Comment 7
2015-04-27 12:54:35 PDT
Created
attachment 251772
[details]
Simplify ChangeLog entry
Basile Clement
Comment 8
2015-05-01 14:46:05 PDT
Created
attachment 252172
[details]
Patch for landing
WebKit Commit Bot
Comment 9
2015-05-01 15:34:49 PDT
Comment on
attachment 252172
[details]
Patch for landing Clearing flags on attachment: 252172 Committed
r183691
: <
http://trac.webkit.org/changeset/183691
>
WebKit Commit Bot
Comment 10
2015-05-01 15:34:53 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug