Closed Bug 668292 Opened 10 years ago Closed 10 years ago

IonMonkey: Implement resolution phase of linear scan register allocator

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, 1 obsolete file)

Tracking bug for associated FIXME in the code.
Assignee: general → adrake
Blocks: 657816
Attached patch Patch v0 (obsolete) — Splinter Review
This patch rolls a couple things together:

- Adds control flow resolution
- Opportunistic dead-phi removal
- Fixes a bug in split phi intervals
- Handle REDEFINED policies

I could split the last two out into their own bugs if you want, but they're small and the fixes were needed to test this patch anyway.
Attachment #544420 - Flags: review?(dvander)
Attached patch Patch v1Splinter Review
This one includes a blob that accidentally got merged into another patch in my queue.
Attachment #544420 - Attachment is obsolete: true
Attachment #544426 - Flags: review?(dvander)
Attachment #544420 - Flags: review?(dvander)
Comment on attachment 544426 [details] [diff] [review]
Patch v1

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

::: js/src/ion/IonLIR.cpp
@@ +60,5 @@
>          return phis_[0]->id();
> +    } else {
> +        for (LInstructionIterator i(instructions_.begin()); i != instructions_.end(); i++) {
> +            if (i->id())
> +                return i->id();

When would there not be an end id?

::: js/src/ion/RegisterAllocator.cpp
@@ +255,4 @@
>  RegisterAllocator::createDataStructures()
>  {
>      allowedRegs = RegisterSet::All();
> +    liveIn = lir->mir()->allocate<BitSet*>(graph.numBlocks());

This is fallible.

@@ +268,5 @@
>              if (ins->numDefs()) {
>                  for (size_t j = 0; j < ins->numDefs(); j++) {
> +                    if (ins->getDef(j)->policy() != LDefinition::REDEFINED) {
> +                        uint32 reg = ins->getDef(j)->virtualRegister();
> +                        if (!vregs[reg].init(reg, b, *ins, ins->getDef(j)))

Another case where overloading [] to take a def or use might be nice.
Attachment #544426 - Flags: review?(dvander) → review+
(In reply to comment #3)
> Comment on attachment 544426 [details] [diff] [review] [review]
> Patch v1
> 
> Review of attachment 544426 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/ion/IonLIR.cpp
> @@ +60,5 @@
> >          return phis_[0]->id();
> > +    } else {
> > +        for (LInstructionIterator i(instructions_.begin()); i != instructions_.end(); i++) {
> > +            if (i->id())
> > +                return i->id();
> 
> When would there not be an end id?

Never, because it doesn't make sense to append instructions after the end-of-block control flow. I've replaced the loop with the appropriate asserts.
http://hg.mozilla.org/users/danderson_mozilla.com/ionmonkey/rev/e3cb37bcc0e1
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.