Closed Bug 1266095 Opened 6 years ago Closed 6 years ago

Add option to disable recover ins

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: h4writer, Assigned: h4writer)

Details

Attachments

(1 file)

We have no switch to enable/disable recover instructions. Something I found quite annoying last time around. Also makes it harder to compare things or sometimes makes it possible to remove blame on a part of the code. Also it helped a bit to visualize which parts need it to work.
Attached patch PatchSplinter Review
Assignee: nobody → hv1989
Attachment #8743296 - Flags: review?(nicolas.b.pierron)
Comment on attachment 8743296 [details] [diff] [review]
Patch

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

This patch looks good overall, I will still suggest that you pay attention to Bug 1004527, as it relies on the fact that some resume point operands can be recovered on bailout, instead of being optimized-out.

::: js/src/jit/Lowering.cpp
@@ +4673,5 @@
>  bool
>  LIRGenerator::visitInstruction(MInstruction* ins)
>  {
> +    if (ins->isRecoveredOnBailout()) {
> +        MOZ_ASSERT(!JitOptions.disableRecoverIns);

This will assert on tests cases which are using "assertRecoveredOnBailout", add a guard to skip the test case if this Jit option is enabled.
Attachment #8743296 - Flags: review?(nicolas.b.pierron) → review+
(In reply to Nicolas B. Pierron [:nbp] from comment #2)
> > +    if (ins->isRecoveredOnBailout()) {
> > +        MOZ_ASSERT(!JitOptions.disableRecoverIns);
> 
> This will assert on tests cases which are using "assertRecoveredOnBailout",
> add a guard to skip the test case if this Jit option is enabled.

Or forbid the inlining of MAssertRecoveredOnBailout, which might be simpler.
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1df39fd00dc69598ac688382889a0f9cc738aab
Bug 1266095: IonMonkey - Add possibility to disable recover instructions, r=nbp
https://hg.mozilla.org/mozilla-central/rev/c1df39fd00dc
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.