Closed Bug 1125837 Opened 5 years ago Closed 5 years ago

Merge SimulatorRuntime into Simulator (ARM, probably MIPS)

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: lth, Assigned: lth)

References

Details

Attachments

(1 file)

From https://bugzilla.mozilla.org/show_bug.cgi?id=1118604#c10, this is fallout from removing PJS and moving per-thread fields back into JSRuntime:

We can now simplify the simulator(s) by merging Simulator and SimulatorRuntime. I think we can just move everything in SimulatorRuntime to Simulator. Maybe we can then rename CreateSimulatorRuntime to CreateSimulator, that way we can assume we always have a non-null simulator in Simulator::Current().

We can also remove AutoLockSimulatorRuntime, might be nice for performance.
Assignee: nobody → lhansen
See Also: → 1126745
Pretty straightforward.  I moved the stack limit field out of JSRuntime into the simulator too, so the simulators are pretty self-contained now.

(I'm not real happy about leaving Simulator::Simulator() and Simulator::~Simulator() public but didn't yet find a simple way to hide them while making them accessible to the memory management, and it doesn't seem like a big deal.)
Attachment #8555800 - Flags: review?(jdemooij)
Comment on attachment 8555800 [details] [diff] [review]
Smush SimulatorRuntime into Simulator on ARM and MIPS

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

Very nice.

::: js/src/jit/arm/Simulator-arm.cpp
@@ +1120,5 @@
>      ABIFunctionType type() const { return type_; }
>  
>      static Redirection *Get(void *nativeFunction, ABIFunctionType type) {
>          PerThreadData *pt = TlsPerThreadData.get();
> +        Simulator *sim = pt->simulator();

Nit: I think we can remove |pt| and make this:

Simulator *sim = Simulator::Current();
Attachment #8555800 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/b0e9b9113cb0
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.