Closed
Bug 772255
Opened 12 years ago
Closed 12 years ago
IonMonkey: LCreateThis should accept constant operands
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: sstangl, Unassigned)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file)
2.83 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
LCreateThis currently takes operands for the callee and the prototype. This causes generated code to assume the shape below, for repetitive invocations of LCreateThis on the same function:
> LPointer X
> LPointer Y
> LCreateThis (X, Y) -> Z
> LPointer X
> LPointer Y
> LCreateThis (X, Y) -> Z
> LPointer X
> LPointer Y
> LCreateThis (X, Y) -> Z
These pointers are only necessary for the slow callVM path, and they must be compile-time constants.
We could save on these loads by using local, manual register allocation and movePtr() in visitCreateThisVMCall() after the call to saveLive(). GC can be appeased via ImmGCPtr.
Reporter | ||
Updated•12 years ago
|
Summary: IonMonkey: LCreateThis should not take operands → IonMonkey: LCreateThis should accept constant operands
Reporter | ||
Comment 1•12 years ago
|
||
Permits LCreateThis to bake in constants rather than depend on LPointer. Minor performance gain, probably mattering more in artificially tight loops involving allocation.
Attachment #640458 -
Flags: review?(dvander)
Comment on attachment 640458 [details] [diff] [review] patch Review of attachment 640458 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/CodeGenerator.cpp @@ +1441,5 @@ > + GeneralRegisterSet regs = GeneralRegisterSet::All(); > + if (proto->isRegister() && regs.has(ToRegister(proto))) > + regs.take(ToRegister(proto)); > + if (callee->isRegister() && regs.has(ToRegister(callee))) > + regs.take(ToRegister(callee)); Why wouldn't it have the proto/callee regs? (you could takeUnchecked if proto might == callee but that sounds impossible) @@ +1447,5 @@ > + > + // Push arguments. > + if (proto->isConstant()) { > + masm.movePtr(ImmGCPtr(&proto->toConstant()->toObject()), scratch); > + pushArg(scratch); I don't think you need a scratch register, you can pushArg() an ImmGCPtr.
Attachment #640458 -
Flags: review?(dvander) → review+
Reporter | ||
Comment 3•12 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/61bb4c14d50d
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•