Closed Bug 925088 Opened 6 years ago Closed 6 years ago

Miscellaneous SpiderMonkey micro-optimizations

Categories

(Core :: JavaScript Engine: JIT, enhancement)

x86
All
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: sunfish, Assigned: sunfish)

Details

Attachments

(3 files)

The following are a few micro-optimization patches focused on x86 and x64.
Attachment #815060 - Flags: review?(jdemooij)
Attachment #815061 - Flags: review?(mrosenberg)
Attachment #815064 - Flags: review?(nicolas.b.pierron)
Comment on attachment 815061 [details] [diff] [review]
micro-test-string-truthy.patch

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

wow, the arm backend has done this for a whle.  Guess I should have checked x64.
Attachment #815061 - Flags: review?(mrosenberg) → review+
Comment on attachment 815064 [details] [diff] [review]
micro-fold-branchTest32.patch

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

::: js/src/jit/IonMacroAssembler.cpp
@@ +901,5 @@
>  }
>  
>  void
>  MacroAssembler::checkInterruptFlagsPar(const Register &tempReg,
>                                              Label *fail)

unrelated-nit: indent the second argument correctly.
Attachment #815064 - Flags: review?(nicolas.b.pierron) → review+
Comment on attachment 815060 [details] [diff] [review]
micro-branch-truncate-double.patch

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

Nice :)

::: js/src/jit/x64/MacroAssembler-x64.h
@@ +1048,1 @@
>          JS_ASSERT(dest != ScratchReg);

Nit: we can remove this assert now.
Attachment #815060 - Flags: review?(jdemooij) → review+
(In reply to Nicolas B. Pierron [:nbp] from comment #4)
> @@ +901,5 @@
> >  }
> >  
> >  void
> >  MacroAssembler::checkInterruptFlagsPar(const Register &tempReg,
> >                                              Label *fail)
> 
> unrelated-nit: indent the second argument correctly.

Done.

(In reply to Jan de Mooij [:jandem] from comment #5)
> @@ +1048,1 @@
> >          JS_ASSERT(dest != ScratchReg);
> 
> Nit: we can remove this assert now.

Done.

https://hg.mozilla.org/integration/mozilla-inbound/rev/00e39c694626
https://hg.mozilla.org/integration/mozilla-inbound/rev/92416820c9fa
https://hg.mozilla.org/integration/mozilla-inbound/rev/44c21dcf1274
Could this bug or bug 924642 have regressed Octane-Splay?
(In reply to Guilherme Lima from comment #8)
> Could this bug or bug 924642 have regressed Octane-Splay?

No, see the regression range in Bug 925824.
Splay also regressed 14 hours before the big regression that also affected Raytrace. This specific regression only hits Firefox without GGC.
(In reply to Guilherme Lima from comment #10)
> Splay also regressed 14 hours before the big regression that also affected
> Raytrace. This specific regression only hits Firefox without GGC.

Yes. AWFY doesn't seem to be updating at the moment, but graphs.mozilla.org has several graphs which show that it has not fully recovered. I filed bug 926514 to track this.
You pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/188f87037857 reverting something from here for some reason.

I pushed http://hg.mozilla.org/integration/mozilla-inbound/rev/2a0c1a40594f reverting your revert because you made parallel/timeout-gc.js and parallel/timeout.js timeout.
The reverts are for bug 926514. I'll make that explicit in revert commit messages going forward.
You need to log in before you can comment on or make changes to this bug.