More register-allocation refactoring

RESOLVED FIXED in mozilla35

Status

()

enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sunfish, Assigned: sunfish)

Tracking

unspecified
mozilla35
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments)

The following patch series continues my series on register-allocation-related refactoring. It contains some minor memory-usage reductions and code simplifications, and it is also taking more preparatory steps for some larger changes that I'm working on.
Attachment #8478360 - Flags: review?(bhackett1024)
In the spirit of bug 1020455, this patch allocates LBlocks in an array, instead of having an array of pointers to individually-allocated LBlocks.
Attachment #8478365 - Flags: review?(sstangl)
This patch moves the block_, inputMoves_, and movesAfter_ fields out of the InstructionData sidecar and into LInstruction itself. Since both our major register allocators use InstructionData, there isn't a lot of reason to keep this information where it's less convenient to access.
Attachment #8478371 - Flags: review?(bhackett1024)
With the previous patch making LInstructions know their LBlocks, it's no longer necessary for the VirtualRegister class to store the block itself.
Attachment #8478373 - Flags: review?(bhackett1024)
This patch factors out a base class from LInstruction and makes LPhi derive from it instead of from LInstruction. This makes LPhi smaller, since it doesn't need move group or snapshot fields and helps clarify that LBlock's begin(),end() range doesn't include the LPhis. I'm not particularly attached to the name LElement, so feel free to suggest something different if you like.

This change is similar to how MDefinition is a base class between MInstruction and MPhi. I chose not use use LDefinition in the LIR version because that name already means something else, and because LInstructions can produce multiple values.
Attachment #8478381 - Flags: review?(bhackett1024)
The loops in inputOf and outputOf to skip phi nodes (introduced in bug) turn out to be noticeable when profiling the backend on testcases containing lots of phis. This patch takes advantage of the earlier patch which moves the block field to LInstruction, to find the first or last phi in a group without a loop.
Attachment #8478385 - Flags: review?(bhackett1024)
This patch makes getInputMoveGroup and getMoveGroupAfter take an LInstruction directly, rather than an id or CodePosition, as most of its call sites have an LInstruction already.
Attachment #8478386 - Flags: review?(bhackett1024)
Attachment #8478360 - Flags: review?(bhackett1024) → review+
Attachment #8478371 - Flags: review?(bhackett1024) → review+
Attachment #8478373 - Flags: review?(bhackett1024) → review+
Comment on attachment 8478381 [details] [diff] [review]
lir-lelement.patch

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

Maybe LNode instead of LElement, to match MNode?  'Element' appears in the name of a lot of instructions too, so LElement might be a bit confusing.
Attachment #8478381 - Flags: review?(bhackett1024) → review+
Attachment #8478385 - Flags: review?(bhackett1024) → review+
Attachment #8478386 - Flags: review?(bhackett1024) → review+
Comment on attachment 8478365 [details] [diff] [review]
lir-lblocks.patch

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

::: js/src/jit/LIR.h
@@ +1534,5 @@
>      }
>      uint32_t numBlockIds() const {
>          return mir_.numBlockIds();
>      }
> +    bool initBlock(MBasicBlock *block) {

Prefer "MBasicBlock *mir".
Attachment #8478365 - Flags: review?(sstangl) → review+
You need to log in before you can comment on or make changes to this bug.