Closed Bug 670635 Opened 13 years ago Closed 13 years ago

IonMonkey: Add move groups to register allocators

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: adrake, Assigned: adrake)

References

Details

Attachments

(1 file, 3 obsolete files)

Instead of sticking multi-moves into the instruction stream, we can build them up externally, run a constraint solver on them, and then insert the ordered moves into the instruction stream. The constraint solver can be shared between both register allocators.
Attached patch WIP v0 (obsolete) — Splinter Review
I still need to add support for float registers, but I think this is otherwise mostly finished. Thoughts/suggestions would be appreciated!
Attachment #545163 - Flags: feedback?(dvander)
Attached patch Patch v1 (obsolete) — Splinter Review
The only really ugly part is figuring out whether something is a double, I basically just pick a member of the cycle and check if its allocation is a float reg or a double slot.
Attachment #545163 - Attachment is obsolete: true
Attachment #545173 - Flags: review?(dvander)
Attachment #545163 - Flags: feedback?(dvander)
Comment on attachment 545173 [details] [diff] [review]
Patch v1

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

::: js/src/ion/MoveGroup.cpp
@@ +51,5 @@
> +MoveGroup::toInstructionsBefore(LBlock *block, LInstruction *ins, uint32 stack)
> +{
> +    Vector<Entry, 0, IonAllocPolicy> workStack;
> +
> +    while (!entries_.empty()) {

A comment explaining how the algorithm works would be good.

@@ +65,5 @@
> +            if (IonSpewEnabled(IonSpew_LSRA)) {
> +                fprintf(IonSpewFile, "  Chasing ");
> +                LAllocation::PrintAllocation(IonSpewFile, current.to);
> +                fprintf(IonSpewFile, "\n");
> +            }

Could the spew blocks be separated out from the main algorithm? It makes a little harder to read otherwise, since you have to dig through the spew to see if there's anything important.

@@ +89,5 @@
> +                fprintf(IonSpewFile, " -> ");
> +                LAllocation::PrintAllocation(IonSpewFile, i->to);
> +                fprintf(IonSpewFile, "\n");
> +            }
> +        }

Here too.
Attachment #545173 - Flags: review?(dvander) → review+
Attached patch Patch v2 (obsolete) — Splinter Review
Fixed a stupid thinko.
Attachment #545173 - Attachment is obsolete: true
Attachment #545195 - Flags: review?(dvander)
Attachment #545195 - Flags: review?(dvander)
Attached patch Patch v3Splinter Review
Add comments, kill useless spew, and and break out useful spew into its own function.
Attachment #545195 - Attachment is obsolete: true
Attachment #545201 - Flags: review?(dvander)
Attachment #545201 - Flags: review?(dvander) → review+
http://hg.mozilla.org/users/danderson_mozilla.com/ionmonkey/rev/796a897c4b9f
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.