Issue 10825282: Put PushArgument into the environment instead of raw values.

Issue 10825282: Put PushArgument into the environment instead of raw values. (Closed)

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

Modified:
8 years, 4 months ago

Reviewers:
Florian Schneider, srdjan

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Put PushArgument into the environment instead of raw values. This allows to shorten live ranges and avoid spilling when PushArgument is the last real use of the value. BUG= Committed: https://code.google.com/p/dart/source/detail?r=10508

Patch Set 1 #

Total comments: 2

Patch Set 2 : address Srdjan's comment #

Patch Set 3 : make sure that it works with new deoptimization infrastructure #

Patch Set 4 : revert changes in deopt instructions #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+145 lines, -78 lines)			Patch
M	runtime/vm/code_generator.cc	View	1 2	1 chunk	+5 lines, -2 lines	0 comments	Download
M	runtime/vm/flow_graph_allocator.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	runtime/vm/flow_graph_allocator.cc	View	1	4 chunks	+65 lines, -11 lines	0 comments	Download
M	runtime/vm/flow_graph_builder.cc	View	1 2	2 chunks	+2 lines, -2 lines	0 comments	Download
M	runtime/vm/flow_graph_compiler.cc	View	1 2	1 chunk	+11 lines, -1 line	0 comments	Download
M	runtime/vm/flow_graph_compiler_x64.cc	View	1 2	1 chunk	+1 line, -0 lines	0 comments	Download
M	runtime/vm/il_printer.cc	View	1 2	1 chunk	+1 line, -3 lines	0 comments	Download
M	runtime/vm/intermediate_language.h	View	1 2	4 chunks	+44 lines, -17 lines	0 comments	Download
M	runtime/vm/intermediate_language.cc	View	1 2	2 chunks	+5 lines, -32 lines	0 comments	Download
M	runtime/vm/intermediate_language_ia32.cc	View	1 2	2 chunks	+5 lines, -5 lines	0 comments	Download
M	runtime/vm/intermediate_language_x64.cc	View	1 2	2 chunks	+5 lines, -5 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

srdjan

LGTM with a comment. Check out: https://chromiumcodereview.appspot.com/10825236/ https://chromiumcodereview.appspot.com/10825282/diff/1/runtime/vm/flow_graph_allocator.cc File runtime/vm/flow_graph_allocator.cc (right): https://chromiumcodereview.appspot.com/10825282/diff/1/runtime/vm/flow_graph_allocator.cc#newcode157 runtime/vm/flow_graph_allocator.cc:157: Value* val ...

8 years, 4 months ago (2012-08-09 20:25:26 UTC) #2

Vyacheslav Egorov (Google)

8 years, 4 months ago (2012-08-10 14:27:46 UTC) #3

Thanks for the review. Landing.

I reverted changes in deopt_instructions.{cc,h} and move locations assignment
for PushArguments up to CreateDeoptInfo (previous code was incorrect because
values are added in reverse order).

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

https://chromiumcodereview.appspot.com/10825282/diff/1/runtime/vm/flow_graph_...
runtime/vm/flow_graph_allocator.cc:157: Value* val = values[j];
On 2012/08/09 20:25:26, srdjan wrote:
> I think this looks simpler :
> 
> UseVal* use_val = values[j]->AsUse();
> if ((use_val != NULL) && !use_val->definition()-IsPushArgument()) {
>   live_in->Add(use_val->definition()->ssa_temp_index());
> }

Agreed.

Expand Messages | Collapse Messages