Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(162)

Issue 11418135: Heuristically predict interference on the back edge and use it to minimize number of register reshu… (Closed)

Created:
8 years, 1 month ago by Vyacheslav Egorov (Google)
Modified:
8 years, 1 month ago
Reviewers:
Florian Schneider
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Heuristically predict interference on the back edge and use it to minimize number of register reshuffling which is especially expensive when cycles of XMM registers arise. When allocating free register check if hint can potentially interfere on the back edge try ignoring hint and search for a better candidate. Additionally fix handling of constants in the liveness analysis to accommodate constants used as immediates. R=fschneider@google.com BUG= Committed: https://code.google.com/p/dart/source/detail?r=15291

Patch Set 1 #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+256 lines, -35 lines) Patch
M runtime/vm/flow_graph_allocator.h View 7 chunks +50 lines, -3 lines 0 comments Download
M runtime/vm/flow_graph_allocator.cc View 21 chunks +187 lines, -25 lines 8 comments Download
M runtime/vm/intermediate_language.h View 3 chunks +12 lines, -1 line 0 comments Download
M runtime/vm/intermediate_language.cc View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/locations.h View 2 chunks +6 lines, -6 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Vyacheslav Egorov (Google)
8 years, 1 month ago (2012-11-23 12:35:48 UTC) #1
Florian Schneider
LGTM. https://codereview.chromium.org/11418135/diff/1/runtime/vm/flow_graph_allocator.cc File runtime/vm/flow_graph_allocator.cc (right): https://codereview.chromium.org/11418135/diff/1/runtime/vm/flow_graph_allocator.cc#newcode1690 runtime/vm/flow_graph_allocator.cc:1690: Definition* input = phi->InputAt(i)->definition(); Or shorter: input_phi = ...
8 years, 1 month ago (2012-11-23 13:58:05 UTC) #2
Vyacheslav Egorov (Google)
8 years, 1 month ago (2012-11-23 15:38:53 UTC) #3
Thanks for the review. Landing.

https://codereview.chromium.org/11418135/diff/1/runtime/vm/flow_graph_allocat...
File runtime/vm/flow_graph_allocator.cc (right):

https://codereview.chromium.org/11418135/diff/1/runtime/vm/flow_graph_allocat...
runtime/vm/flow_graph_allocator.cc:1690: Definition* input =
phi->InputAt(i)->definition();
On 2012/11/23 13:58:06, Florian Schneider wrote:
> Or shorter:
> 
> input_phi = phi->InputAt(i)->definition()->AsPhi();
> if (input_phi != NULL) { ...

Done.

https://codereview.chromium.org/11418135/diff/1/runtime/vm/flow_graph_allocat...
runtime/vm/flow_graph_allocator.cc:1743: // If we are in the loop try to reduce
number of moves on the back edge by
On 2012/11/23 13:58:06, Florian Schneider wrote:
> Which loop? s/the/a/?

Done.

https://codereview.chromium.org/11418135/diff/1/runtime/vm/flow_graph_allocat...
runtime/vm/flow_graph_allocator.cc:1751:
ASSERT(static_cast<intptr_t>(kNumberOfXmmRegisters) <=
On 2012/11/23 13:58:06, Florian Schneider wrote:
> COMPILE_ASSERT?

COMPILE_ASSERT seems to be available only in debug.

https://codereview.chromium.org/11418135/diff/1/runtime/vm/flow_graph_allocat...
runtime/vm/flow_graph_allocator.cc:1765: if ((unallocated->vreg() >= 0) &&
On 2012/11/23 13:58:06, Florian Schneider wrote:
> This condition is already tested in line 1747.

Done.

Powered by Google App Engine
This is Rietveld 408576698