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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Chris Leary [:cdleary] (not checking bugmail)]

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.

Comment 1

[Chris Leary [:cdleary] (not checking bugmail)]

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.

Comment 2

[Chris Leary [:cdleary] (not checking bugmail)]

(In reply to comment #1)
> by the end of the day.

Or... in the next few minutes.

Comment 3

[Chris Leary [:cdleary] (not checking bugmail)]

Attachment: Ensure space for the inline path of the ICs.

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.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

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.

Comment 5

[Chris Leary [:cdleary] (not checking bugmail)]

(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.

Comment 6

[Chris Leary [:cdleary] (not checking bugmail)]

Attachment: [Updated] Ensure space for the inline path of the ICs.

Checkin needed.

Comment 7

[Chris Leary [:cdleary] (not checking bugmail)]

Whoops, that's a keyword, not a whiteboard thing.

Comment 8

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mozilla-central/rev/50795657150e