Reviewers: titzer, Benedikt Meurer, jarin,
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/coalesced-live-ranges.h
File src/compiler/coalesced-live-ranges.h (right):
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/coalesced-live-ranges.h#newcode16
src/compiler/coalesced-live-ranges.h:16: // TODO(mtrofin): we may be
calculating the weight multiple times. Consider
On 2015/06/25 04:41:06, Benedikt Meurer wrote:
I'd prefer to have it on the LiveRange instead. This utility class
doesn't seem
to make things easier for now.
Done.
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/coalesced-live-ranges.h#newcode18
src/compiler/coalesced-live-ranges.h:18: struct AllocatedRange {
On 2015/06/24 23:00:07, titzer wrote:
WeightedLiveRange? Should be a class if it has private members.
Removed the type, by virtue of moving weight to LiveRange.
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/greedy-allocator.h
File src/compiler/greedy-allocator.h (right):
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/greedy-allocator.h#newcode25
src/compiler/greedy-allocator.h:25: }
On 2015/06/25 04:41:06, Benedikt Meurer wrote:
Nit: Please add an appropriate operator> if you decide to overload
operator<,
even if it's currently unused.
Done.
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/greedy-allocator.h#newcode68
src/compiler/greedy-allocator.h:68: void InitializeAllocations();
On 2015/06/24 23:05:44, titzer wrote:
Why not just name the method after what it does? InsertFixedRanges()?
Good suggestion. I'm proposing PreallocateFixedRanges.
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/greedy-allocator.h#newcode72
src/compiler/greedy-allocator.h:72: void InitializeScheduler();
On 2015/06/24 23:05:44, titzer wrote:
Same here.
Done.
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/greedy-allocator.h#newcode78
src/compiler/greedy-allocator.h:78: void ProcessCandidate(const
AllocationCandidate& candidate);
On 2015/06/24 23:05:44, titzer wrote:
What does Process mean?
Done.
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/greedy-allocator.h#newcode86
src/compiler/greedy-allocator.h:86: float
GetCandidateRangeWeight(LiveRange* range, unsigned size);
On 2015/06/24 23:05:44, titzer wrote:
Even though this is camel case (and thus CouldPerformCalculations), it
might be
better to avoid naming things with the Get prefix if they compute
something.
Done by virtue of having had changed a bit more on the API.
https://codereview.chromium.org/1205173002/diff/20001/src/compiler/greedy-allocator.h#newcode93
src/compiler/greedy-allocator.h:93: bool HandleSpillOperands(LiveRange*
range);
On 2015/06/24 23:05:44, titzer wrote:
Again, no idea what "Handle" means here.
Done.
https://codereview.chromium.org/1205173002/diff/40001/tools/gyp/v8.gyp
File tools/gyp/v8.gyp (right):
https://codereview.chromium.org/1205173002/diff/40001/tools/gyp/v8.gyp#newcode572
tools/gyp/v8.gyp:572:
'../../src/compiler/simplified-operator-reducer.h',
I rebased before resubmitting.
Description:
[turbofan] Greedy allocator refactoring.
Separated core greedy allocator concepts, exposing the APIs we would want to
continue working with. In particular, this change completely reworks
CoalescedLiveRanges to reflect the fact that we expect more than one
possible
conflict, scrapping the initial design of the structure. Since this is a
critical part of the design, this change may be thought of as a full
rewrite of
the algorithm.
Reduced all heuristics to just 2 essential ones: split "somewhere", which
we'll
still need when all other heuristics fail; and spill.
Introduced a simple primitive for splitting - at GapPosition::START. The
goal is
to use such primitives to quickly and reliably author heuristics.
I expected this primitive to "just work" for any arbitrary instruction index
within a live range - e.g. its middle. That's not the case, it seems to
upset
execution in certain scenarios. Restricting to either before/after use
positions
seems to work. I'm still investigating what the source of failures is in the
case of "arbitrary instruction in the range" case.
I intended to document the rationale and prove the soundness of always using
START for splits, but I will postpone to after this last remaining issue is
resolved.
Please review this at https://codereview.chromium.org/1205173002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+592, -366 lines):
M BUILD.gn
A src/compiler/coalesced-live-ranges.h
A src/compiler/coalesced-live-ranges.cc
M src/compiler/greedy-allocator.h
M src/compiler/greedy-allocator.cc
M src/compiler/register-allocator.h
M src/compiler/register-allocator.cc
M tools/gyp/v8.gyp
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
