Issue 10562041: Introduce HLocalValue and HLocalGet/Set.

Issue 10562041: Introduce HLocalValue and HLocalGet/Set. (Closed)

Created:
8 years, 6 months ago by floitsch

Modified:
8 years, 6 months ago

Reviewers:
Lasse Reichstein Nielsen, kasperl

CC:
reviews_dartlang.org, ngeoffray

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

Visibility:
Public.

More Reviews

Description

Introduce HLocalValue and HLocalGet/Set. Committed: https://code.google.com/p/dart/source/detail?r=8854

Patch Set 2 : Address comments. #

Created: 8 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+107 lines, -50 lines)			Patch
M	lib/compiler/implementation/ssa/builder.dart	View		4 chunks	+8 lines, -8 lines	0 comments	Download
M	lib/compiler/implementation/ssa/codegen.dart	View	1	2 chunks	+31 lines, -30 lines	0 comments	Download
M	lib/compiler/implementation/ssa/nodes.dart	View	1	6 chunks	+56 lines, -7 lines	0 comments	Download
M	lib/compiler/implementation/ssa/tracer.dart	View		2 chunks	+7 lines, -0 lines	0 comments	Download
M	lib/compiler/implementation/ssa/variable_allocator.dart	View	1	2 chunks	+5 lines, -5 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

floitsch

In follow-up CLs I hope to keep all variables in the builder, and create the ...

8 years, 6 months ago (2012-06-18 18:21:33 UTC) #1

kasperl

LGTM. https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implementation/ssa/codegen.dart File lib/compiler/implementation/ssa/codegen.dart (right): https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implementation/ssa/codegen.dart#newcode1731 lib/compiler/implementation/ssa/codegen.dart:1731: String name; String name = https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implementation/ssa/codegen.dart#newcode1751 lib/compiler/implementation/ssa/codegen.dart:1751: String ...

8 years, 6 months ago (2012-06-19 11:19:44 UTC) #2

Lasse Reichstein Nielsen

What Kasper said! LGTM https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implementation/ssa/nodes.dart File lib/compiler/implementation/ssa/nodes.dart (right): https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implementation/ssa/nodes.dart#newcode1862 lib/compiler/implementation/ssa/nodes.dart:1862: * An [HLocalValue] represents a ...

8 years, 6 months ago (2012-06-19 11:27:51 UTC) #3

floitsch

8 years, 6 months ago (2012-06-19 11:31:32 UTC) #4

https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1731: String name;
On 2012/06/19 11:19:45, kasperl wrote:
> String name =

Done.

https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1751: String name;
On 2012/06/19 11:19:45, kasperl wrote:
> Unused name?

Done.

https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1902:
visitParameterValue(HParameterValue node) {
On 2012/06/19 11:19:45, kasperl wrote:
> Remove this method entirely? By default it should call visitLocalValue, right?

This visitor does not extend the base-visitor. We thus have to implement all
methods. Made it redirect to visitLocalValue.

https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:1329: // TODO(floitsch): implement
more fine grain side effects.
On 2012/06/19 11:19:45, kasperl wrote:
> grained

Done.

https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:1862: * An [HLocalValue] represents a
local. Contrary to [HParameterValue]s its
On 2012/06/19 11:27:51, Lasse Reichstein Nielsen wrote:
> Do you mean "contrary", i.e., that a HParameterValue must NOT be used first in
a
> HLocalSet? Or just "unlike"?

Unlike. done.

https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/variable_allocator.dart (right):

https://chromiumcodereview.appspot.com/10562041/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/variable_allocator.dart:567: // but we need a
name for parameters. We should probably not make
On 2012/06/19 11:19:45, kasperl wrote:
> parameters -> them

Done.

Expand Messages | Collapse Messages