ARM: prevent constant pool from being dumped in the middle of MICs

RESOLVED FIXED in mozilla2.0b8

Status

()

defect
--
major
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: cdleary, Assigned: cdleary)

Tracking

unspecified
mozilla2.0b8
ARM
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(fennec2.0b3+)

Details

Attachments

(2 attachments)

We need to port the ENSURE_IC_SPACE mechanism from bug 588021 comment 23 to prevent the same from happening the middle of a MIC, which are currently enabled. Could be causing some random crashers on ARM, but the probability is low because the MICs are small.
Flags: wanted-fennec1.0?
We should probably block fennec b3 on this and let the PICs slip. It's an easy and low-risk patch to ensure IC space, will have it by the end of the day.
tracking-fennec: --- → ?
Flags: wanted-fennec1.0?
(In reply to comment #1)
> by the end of the day.

Or... in the next few minutes.
Conservative estimate might be a little high, but in the worse case we dump the constant pool slightly more than we have to. Not sure if this needs to be done for the call ICs as well, because they appear fairly non-linear.
Attachment #492750 - Flags: review?(dvander)
tracking-fennec: ? → 2.0b3+
Comment on attachment 492750 [details] [diff] [review]
Ensure space for the inline path of the ICs.

>+ * ICs perform repatching on the inline (fast) path by knowing small immediate
>+ * offset values at which patchable instructions reside. Dumping a huge
>+ * constant pool into the middle of an IC's inline path violates its
>+ * invariants, 

Please describe what those invariants are in this comment - that some ICs bake in constants, and if the literal pool is dumped, those won't be valid.

>+  public:
>+    AutoReserveICSpace(Assembler &masm, MICInfo::Kind kind) : masm(masm) {
>+        (void) kind; /* TODO: switch on kind for finer granularity. */

TODO should have a bug number, or just remove the TODO.

Also, I'd put this new class in BaseAssembler.h or BaseCompiler.h instead, those tend to have the platform-specific helpers.
Attachment #492750 - Flags: review?(dvander) → review+
(In reply to comment #4)
> >+ * ICs perform repatching on the inline (fast) path by knowing small immediate
> >+ * offset values at which patchable instructions reside. Dumping a huge
> >+ * constant pool into the middle of an IC's inline path violates its
> >+ * invariants, 
> 
> Please describe what those invariants are in this comment - that some ICs bake
> in constants, and if the literal pool is dumped, those won't be valid.

Will do. The pools are generally large (I've seen them at ~500B) and even exceed the small fixed-width integers we use for recording dynamic offsets.
Whiteboard: checkin-needed
Whoops, that's a keyword, not a whiteboard thing.
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/50795657150e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: checkin-needed
Target Milestone: --- → mozilla2.0b8
Depends on: 615875
You need to log in before you can comment on or make changes to this bug.