Bug 91788

Summary:

Property storage should grow in reverse address direction, to support butterflies

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

barraclough, fpizlo, ggaren, mhahnenberg, oliver, paroga, zherczeg

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

91796, 92086, 92088

Bug Blocks:

91933

Attachments:

Description	Flags
it starts	none
more	none
it does more things than before	none
almost done	none
the patch	ggaren: review+

Description Filip Pizlo 2012-07-19 14:51:28 PDT

This will allow us to implement butterfly objects.

Comment 1 Filip Pizlo 2012-07-19 15:14:37 PDT

Created attachment 153360 [details]
it starts

Comment 2 Filip Pizlo 2012-07-19 20:43:49 PDT

Created attachment 153386 [details]
more

Comment 3 Filip Pizlo 2012-07-19 22:11:50 PDT

Created attachment 153409 [details]
it does more things than before

And now, it can even reallocate storage.

Comment 4 Filip Pizlo 2012-07-21 14:55:43 PDT

I initially wanted to reverse both the direction of storage and the direction of PropertyOffsets in one patch, 'cause that seemed more sensible.  But when it came to debugging (both performance and soundness), I found that the fact that I had conflated these two changes made life totally miserable.  I'm now making only one of the changes (direction of storage) and making it so that if you have to do an out-of-line uncached access, there's some negation that needs to happen.  We can make the other change later if we feel like it.

Comment 5 Filip Pizlo 2012-07-21 14:59:57 PDT

Created attachment 153679 [details]
almost done

Backing up, and putting up for EWS though I suspect the results won't be pretty.

Comment 6 Filip Pizlo 2012-07-21 15:01:18 PDT

Zoltan: this might require some ARM changes because our compact offset patching (for get_by_id) may want to patch in a negative offset.  Note that the patch doesn't yet have the right 32-bit code, so it doesn't make sense to start porting yet.  But I figured I'd give you a heads up.

Comment 7 Filip Pizlo 2012-07-23 17:29:07 PDT

Created attachment 153912 [details]
the patch

Comment 8 Geoffrey Garen 2012-07-23 17:42:21 PDT

Comment on attachment 153912 [details]
the patch

r=me

Comment 9 Filip Pizlo 2012-07-23 19:13:53 PDT

Landed in http://trac.webkit.org/changeset/123417

Comment 10 Zoltan Herczeg 2012-07-24 02:51:26 PDT

> Zoltan: this might require some ARM changes because our compact offset patching (for get_by_id) may want to patch in a negative offset.  Note that the patch doesn't yet have the right 32-bit code, so it doesn't make sense to start porting yet.  But I figured I'd give you a heads up.

Thanks, I will fix it soon.

Comment 11 Patrick R. Gansterer 2012-07-24 12:19:21 PDT

Landed windows build fix in http://trac.webkit.org/changeset/123503