Closed Bug 901081 Opened 6 years ago Closed 6 years ago

IonMonkey: RegisterAllocator_Stupid asserts/crashes on Citadel/bullet.js

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: luke, Assigned: bhackett)

References

()

Details

Attachments

(1 file)

In a sequentil build, register allocation is taking about ~45% of all asm.js compilation time.  I think much of this is from large functions and so I was thinking about, when lifo.used() > X MB, using the RegisterAllocator_Stupid.  Experimenting with this I found that the StupidAllocator crashes on several of the large asm.js workloads.  A quick testcase you can run in the shell is arewefastyet/benchmarks/asmjs-apps/bullet.js.

Side question: should I expect the stupid allocator to be a lot faster?  With a little work would it be able to ship turned on?
The main problem with the stupid allocator in the general case is that it does not do a liveness analysis, which is needed in order to populate safepoints correctly.  It does this population by hooking in to the liveness analysis performed when verifying the integrity of an allocation, which in all likelihood will take longer than LSRA's entire operation.  Now, since iirc asm.js doesn't have safepoints this step could probably just be skipped, and the allocation should be quick.  I'll look at the crash.
Heh, I was just now noticing AllocationIntegrityState in the profile and about to try putting it behind "if (mir->compileAsmJS())".  Thanks for looking into the crash!
Great, with the AllocationIntegrityState commented out, on the asjs-apps that do work: zlib goes from 73ms in regalloc to 10ms, lua_binarytrees goes from 138ms to 51ms, box2d goes from 84ms to 18ms.
Attached patch patchSplinter Review
The stupid allocator gets the register associated with fixed uses in the wrong way, which causes it to treat fixed uses of float registers as if they were fixed uses of general purpose registers.
Assignee: general → bhackett1024
Attachment #785216 - Flags: review?(luke)
Attachment #785216 - Flags: review?(luke) → review+
https://hg.mozilla.org/mozilla-central/rev/c24d2c94d6fb
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.