Preparatory refactoring-only steps for inlining accessors.

Preparatory refactoring-only steps for inlining accessors.

The main change is that the predicate for accessor calls is centralized in
IsAccessorCall now, and that the tests using it are directly in the HandleFoo
and VisitBar functions, not buried deep down in some BuildBaz functions. This is
necessary because later inlining decisions have to be done early, which is in
turn necessary because inlining modifies the current hydrogen graph.

Furthermore, identifier names and argument order have been unified a bit.

[Michael Starzinger, 2012-07-19 13:28:13]

First round of comments.

https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc#newcod...
src/hydrogen.cc:4778: CHECK_ALIVE(store = BuildCallSetter(literal,
For object literals we should never get an accessor because we always use the
internal Object() constructor and duplicate identifiers are not allowed in
literals. So better turn this into an assertion here.

https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc#newcod...
src/hydrogen.cc:5411: }
This part cannot yet be inlined with out current machinery, because it requires
follow-up work after the inlined getter returns. Unfortunately the getter will
be inlined into the outer AST-context, which might be a test-context. In this
case the is no single basic block that can contain the follow-up work.

What we actually want to do is inline the getter into an artificial
value-context (similar to what VisitForValue does), so that we have a single
return block.

https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc#newcod...
src/hydrogen.cc:5688: bool HGraphBuilder::IsAccessorCall(Handle<Map> map,
The name of this method is misleading, because it actually performs a lookup.
Can we call it "LookupAccessorCall" or "LookupAccessorPair" instead?

https://chromiumcodereview.appspot.com/10801013/diff/1/src/hydrogen.cc#newcod...
src/hydrogen.cc:7849: }
Same comment as for HandleCompoundAssignment() applies here.

[Sven Panne, 2012-07-23 12:25:01]

As discussed offline, I've addressed 2 review comments and experiment with a
simple case of inlining for the rest to see if we're heading into the direction
with this refactoring.

No need to review things right now, I'll ping again when something is actually
working.

http://codereview.chromium.org/10801013/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/10801013/diff/1/src/hydrogen.cc#newcode4778
src/hydrogen.cc:4778: CHECK_ALIVE(store = BuildCallSetter(literal,
On 2012/07/19 13:28:13, Michael Starzinger wrote:
> For object literals we should never get an accessor because we always use the
> internal Object() constructor and duplicate identifiers are not allowed in
> literals. So better turn this into an assertion here.

Done.

http://codereview.chromium.org/10801013/diff/1/src/hydrogen.cc#newcode5688
src/hydrogen.cc:5688: bool HGraphBuilder::IsAccessorCall(Handle<Map> map,
On 2012/07/19 13:28:13, Michael Starzinger wrote:
> The name of this method is misleading, because it actually performs a lookup.
> Can we call it "LookupAccessorCall" or "LookupAccessorPair" instead?

Done.