Issue 10826097: Use explicit push-argument for InstanceSetter instruction.

Issue 10826097: Use explicit push-argument for InstanceSetter instruction. (Closed)

Created:
8 years, 4 months ago by Florian Schneider

Modified:
8 years, 4 months ago

Reviewers:
srdjan

CC:
reviews_dartlang.org, vm-dev_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/runtime/

Visibility:
Public.

More Reviews

Description

Use explicit push-argument for InstanceSetter instruction. This allows optimizing functions with InstanceSetter. Committed: https://code.google.com/p/dart/source/detail?r=10157

Patch Set 3 : Fix syntax error #

Total comments: 6

Patch Set 5 : #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+66 lines, -44 lines)			Patch
M	vm/flow_graph_builder.h	View	1 2 3	1 chunk	+4 lines, -3 lines	0 comments	Download
M	vm/flow_graph_builder.cc	View	1 2 3	1 chunk	+25 lines, -16 lines	0 comments	Download
M	vm/flow_graph_optimizer.cc	View		2 chunks	+6 lines, -4 lines	0 comments	Download
M	vm/il_printer.cc	View		1 chunk	+10 lines, -0 lines	0 comments	Download
M	vm/intermediate_language.h	View		2 chunks	+16 lines, -14 lines	0 comments	Download
M	vm/intermediate_language.cc	View		1 chunk	+3 lines, -1 line	0 comments	Download
M	vm/intermediate_language_ia32.cc	View	1 2 3	1 chunk	+1 line, -3 lines	0 comments	Download
M	vm/intermediate_language_x64.cc	View		1 chunk	+1 line, -3 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

srdjan

LGTM https://chromiumcodereview.appspot.com/10826097/diff/5003/vm/flow_graph_builder.h File vm/flow_graph_builder.h (right): https://chromiumcodereview.appspot.com/10826097/diff/5003/vm/flow_graph_builder.h#newcode199 vm/flow_graph_builder.h:199: bool is_used); Add comment describing what 'is_used' means, ...

8 years, 4 months ago (2012-08-01 16:53:30 UTC) #2

Florian Schneider

8 years, 4 months ago (2012-08-02 10:06:59 UTC) #3

https://chromiumcodereview.appspot.com/10826097/diff/5003/vm/flow_graph_build...
File vm/flow_graph_builder.h (right):

https://chromiumcodereview.appspot.com/10826097/diff/5003/vm/flow_graph_build...
vm/flow_graph_builder.h:199: bool is_used);
On 2012/08/01 16:53:31, srdjan wrote:
> Add comment describing what 'is_used' means, or even better rename it so it is
> self-explanatory, e.g., result_is_needed.

Done.

https://chromiumcodereview.appspot.com/10826097/diff/5003/vm/intermediate_lan...
File vm/intermediate_language.h (right):

https://chromiumcodereview.appspot.com/10826097/diff/5003/vm/intermediate_lan...
vm/intermediate_language.h:1068: ZoneGrowableArray<PushArgumentInstr*>*
arguments)
On 2012/08/01 16:53:31, srdjan wrote:
> It is OK that way, but I wonder why not pass explicitly two arguments. Is it
in
> preparation for converting it to instance call?

As part of introducing push-argument instructions I'm making all call-like
instructions have a uniform structure. I'd like to convert it to an instance
call as a separate CL.

https://chromiumcodereview.appspot.com/10826097/diff/5003/vm/intermediate_lan...
File vm/intermediate_language_x64.cc (right):

https://chromiumcodereview.appspot.com/10826097/diff/5003/vm/intermediate_lan...
vm/intermediate_language_x64.cc:2153: return Computation::MakeCallSummary();
On 2012/08/01 16:53:31, srdjan wrote:
> What about IA32? Let's try to keep it in sync when possible.

Oops. Thanks, I forgot about IA32.

Expand Messages | Collapse Messages