Some IC stubs do a callWithABI to some helper function and save/restore volatile registers. Most platforms have a lot of double registers, so we spend a fair amount of time pushing/popping them. This is silly because (1) Baseline code doesn't have any live double registers so we don't have to save any of them and (2) Ion ICs have a LiveRegisterSet so we only need to save/restore the float registers from that set. Adding a set of live float registers to the CacheIRCompiler base class should be easy and should be a measurable performance win in some cases.

This improves the microbenchmark below from 152 ms to 119 ms so it seems worth it. With --no-ion I get 197 ms to 175 ms. --- function f() { var a = []; for (var i=0; i<3456; i++) a.push(i); var s = "1234"; var res = 0; var t = new Date; for (var i=0; i<10000000; i++) { res = a[s]; } print(new Date - t); return res; } f();

Comment on attachment 8841916 [details] [diff] [review] Patch Review of attachment 8841916 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/BaselineCacheIRCompiler.cpp @@ +816,1 @@ > LiveRegisterSet save(regs.asLiveSet()); nit: regs is only used as a way to init the LiveSet, remove "regs, and initialize the LiveRegisterSet accordingly. @@ +817,5 @@ > > masm.PushRegsInMask(save); > > masm.setupUnalignedABICall(scratch1); > masm.loadJSContext(scratch1); existing-nit: Add a comment that NativeObject::growSlotsDontReportOOM does not GC. @@ +1698,5 @@ > break; > } > > + // Baseline doesn't allocate float registers so none of them are live. > + liveFloatRegs_.set() = FloatRegisterSet(); We should avoid manipulating the ".set()", as this is basically the internal state of the LiveSet, and I should remove this function. I suggest you do the following instead: liveFloatRegs_ = LiveFloatRegisterSet(FloatRegisterSet()); ::: js/src/jit/CacheIRCompiler.cpp @@ +1494,5 @@ > FailurePath* failure; > if (!addFailurePath(&failure)) > return false; > > + AllocatableRegisterSet regs(GeneralRegisterSet::Volatile(), liveVolatileFloatRegs()); nit: regs is only used to initialize the LiveRegisterSet, which has the same constructors. @@ +2131,5 @@ > > Register obj = output.valueReg().scratchReg(); > masm.unboxObject(output.valueReg(), obj); > > + AllocatableRegisterSet regs(GeneralRegisterSet::Volatile(), liveVolatileFloatRegs()); nit: same here. @@ +2139,5 @@ > masm.setupUnalignedABICall(scratch); > masm.loadJSContext(scratch); > masm.passABIArg(scratch); > masm.passABIArg(obj); > masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, WrapObjectPure)); existing-nit: add JS::AutoCheckCannotGC instance in the body of WrapObjectPure. ::: js/src/jit/CacheIRCompiler.h @@ +568,5 @@ > > MOZ_MUST_USE bool addFailurePath(FailurePath** failure); > MOZ_MUST_USE bool emitFailurePath(size_t i); > > + FloatRegisterSet liveVolatileFloatRegs() const { nit: Add a comment that this function should only be used for non-GC calls made with callWithABI. ::: js/src/jit/IonCacheIRCompiler.cpp @@ +411,5 @@ > MOZ_CRASH("Invalid cache"); > } > > + if (liveRegs_) > + liveFloatRegs_.set() = liveRegs_->fpus(); nit: same here, do not use ".set()" to Mutate the internal state, prefer the copy constructor. @@ +600,5 @@ > masm.setupUnalignedABICall(scratch); > masm.movePtr(ImmGCPtr(atom), scratch); > masm.passABIArg(scratch); > masm.passABIArg(str); > masm.callWithABI(JS_FUNC_TO_DATA_PTR(void*, EqualStringsHelper)); existing-nit: add JS::AutoCheckCannotGC instance in the body of EqualStringsHelper. @@ +1096,1 @@ > LiveRegisterSet save(regs.asLiveSet()); existing-nit: Use LiveRegisterSet constructor instead of copying the content of the AllocatableRegisterSet.

Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ee6ba4fcb76f Optimize volatile register spilling for C++ calls from IC stubs. r=nbp