Issue 10828115: Implement simple spill store elimination.

Issue 10828115: Implement simple spill store elimination. (Closed)

Created:
8 years, 4 months ago by Vyacheslav Egorov (Google)

Modified:
8 years, 4 months ago

Reviewers:
srdjan

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Implement simple spill store elimination. Values that are spilled somewhere during their life-time are spilled eagerly at their definiton. Additionally improve liverange printing used for debugging purposes. R=srdjan@google.com BUG= Committed: https://code.google.com/p/dart/source/detail?r=10155

Patch Set 1 #

Total comments: 4

Patch Set 2 : address Srdjan's comments #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+139 lines, -48 lines)			Patch
M	runtime/vm/flow_graph_allocator.h	View	1	8 chunks	+19 lines, -2 lines	0 comments	Download
M	runtime/vm/flow_graph_allocator.cc	View	1	19 chunks	+109 lines, -45 lines	0 comments	Download
M	runtime/vm/locations.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	runtime/vm/locations.cc	View		1 chunk	+10 lines, -1 line	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

srdjan

LGTM https://chromiumcodereview.appspot.com/10828115/diff/1/runtime/vm/flow_graph_allocator.cc File runtime/vm/flow_graph_allocator.cc (right): https://chromiumcodereview.appspot.com/10828115/diff/1/runtime/vm/flow_graph_allocator.cc#newcode363 runtime/vm/flow_graph_allocator.cc:363: range->set_assigned_location(loc); It seems strange to sometimes set assigned ...

8 years, 4 months ago (2012-08-01 18:48:04 UTC) #2

Vyacheslav Egorov (Google)

8 years, 4 months ago (2012-08-02 08:46:09 UTC) #3

Thanks for the review. Landing.

https://chromiumcodereview.appspot.com/10828115/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_allocator.cc (right):

https://chromiumcodereview.appspot.com/10828115/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:363: range->set_assigned_location(loc);
On 2012/08/01 18:48:04, srdjan wrote:
> It seems strange to sometimes set assigned location only in debug mode. How
> about getting rid of DEBUG and adding comment?

Done.

https://chromiumcodereview.appspot.com/10828115/diff/1/runtime/vm/flow_graph_...
File runtime/vm/flow_graph_allocator.h (right):

https://chromiumcodereview.appspot.com/10828115/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.h:195: GrowableArray<LiveRange*> temporaries_;
On 2012/08/01 18:48:04, srdjan wrote:
> We are trying to use the following form everywhere:
> #if defined(DEBUG)
> 
> Question is should we let it be in release mode as well. Not much overhead,
just
> add a comment what it is for.

Done.

Expand Messages | Collapse Messages