CacheIR: Attach DenseElementHole even with zero dense elements

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

(Blocks 1 bug)

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

2 years ago
Posted patch WIP (obsolete) — Splinter Review
Found on twitter again. As long as the object doesn't have indexed properties besides dense elements we should be able to attach this IC even when the initialized length is zero. This basically means there are no indexed elements anywhere on that object.
Assignee

Comment 1

2 years ago
Apparently there is some edge case where this is causing issues: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff705046fbbb21cd440eeb6effc09f4b273fddf9&selectedJob=65747940. Just started debugging.
Assignee

Comment 2

2 years ago
For some reason we are attaching the stub for CData objects (http://searchfox.org/mozilla-central/source/js/src/ctypes/CTypes.cpp#583) which should be impossible, because that class has a getProperty ClassOps, which we check for in ClassCanHaveExtraProperties. Compiler bug?
Assignee

Comment 3

2 years ago
Somehow I didn't spot the difference between getProperty in ClassOps and ObjectOps. 

Anyway I am asking Brian for additional review. What we are trying to accomplish here is make sure that indexed properties would either be defined as a dense element or not exist at all. This also means if the object currently has zero dense elements we have to make sure there is no other way for indexed properties to appear.
Assignee: nobody → evilpies
Attachment #8822943 - Attachment is obsolete: true
Attachment #8823421 - Flags: review?(jdemooij)
Attachment #8823421 - Flags: review?(bhackett1024)
Comment on attachment 8823421 [details] [diff] [review]
v1 Attach dense element hole IC even with zero dense elements

Review of attachment 8823421 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.
Attachment #8823421 - Flags: review?(jdemooij) → review+
Attachment #8823421 - Flags: review?(bhackett1024) → review+

Comment 5

2 years ago
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f897399fb28a
Attach dense element hole IC even with zero dense elements. r=jandem,bhackett

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f897399fb28a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.