Closed
Bug 1058095
Opened 10 years ago
Closed 10 years ago
More register-allocation refactoring
Categories
(Core :: JavaScript Engine: JIT, enhancement)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: sunfish, Assigned: sunfish)
Details
Attachments
(7 files)
2.79 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
8.76 KB,
patch
|
sstangl
:
review+
|
Details | Diff | Splinter Review |
25.38 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
3.35 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
38.45 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
16.84 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
15.71 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
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)
Assignee | ||
Comment 1•10 years ago
|
||
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)
Assignee | ||
Comment 2•10 years ago
|
||
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)
Assignee | ||
Comment 3•10 years ago
|
||
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)
Assignee | ||
Comment 4•10 years ago
|
||
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)
Assignee | ||
Comment 5•10 years ago
|
||
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)
Assignee | ||
Comment 6•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8478360 -
Flags: review?(bhackett1024) → review+
Updated•10 years ago
|
Attachment #8478371 -
Flags: review?(bhackett1024) → review+
Updated•10 years ago
|
Attachment #8478373 -
Flags: review?(bhackett1024) → review+
Comment 7•10 years ago
|
||
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+
Updated•10 years ago
|
Attachment #8478385 -
Flags: review?(bhackett1024) → review+
Updated•10 years ago
|
Attachment #8478386 -
Flags: review?(bhackett1024) → review+
Comment 8•10 years ago
|
||
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+
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d5ddfdf72d6 https://hg.mozilla.org/integration/mozilla-inbound/rev/7deba8fa4e19 https://hg.mozilla.org/integration/mozilla-inbound/rev/bd25766ac048 https://hg.mozilla.org/integration/mozilla-inbound/rev/8843395921bf https://hg.mozilla.org/integration/mozilla-inbound/rev/ba2a7e2ec1a0 https://hg.mozilla.org/integration/mozilla-inbound/rev/80c1cb537478 https://hg.mozilla.org/integration/mozilla-inbound/rev/204dc904b8c0
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3d5ddfdf72d6 https://hg.mozilla.org/mozilla-central/rev/7deba8fa4e19 https://hg.mozilla.org/mozilla-central/rev/bd25766ac048 https://hg.mozilla.org/mozilla-central/rev/8843395921bf https://hg.mozilla.org/mozilla-central/rev/ba2a7e2ec1a0 https://hg.mozilla.org/mozilla-central/rev/80c1cb537478 https://hg.mozilla.org/mozilla-central/rev/204dc904b8c0
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•