Inline simple setter calls.

Currently only simple setter calls are handled (i.e. no calls in count
operations or compound assignments), and deoptimization in the setter is not
handled at all. Because of the latter, we temporarily hide this feature behind
the --inline-accessors flag, just like inlining getters.

We now use an enum everywhere we depend on the handling of a return value,
passing around several boolean would be more confusing.

Made VisitReturnStatement and the final parts of TryInline more similar, so
matching them visually is a bit easier now.

Simplified the signature of AddLeaveInlined, the target of the HGoto can simply
be retrieved from the function state.

Review URL: https://chromiumcodereview.appspot.com/10836133

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12286 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/lithium-arm.cc

@@ -2239,7 +2239,7 @@ LInstruction* LChunkBuilder::DoEnterInlined(HEnterInlined* instr) {
                                                instr->function(),
                                                undefined,
                                                instr->call_kind(),
-                                               instr->is_construct());
+                                               instr->inlining_kind());
   if (instr->arguments_var() != NULL) {
     inner->Bind(instr->arguments_var(), graph()->GetArgumentsObject());
   }

src/arm/lithium-codegen-arm.cc

@@ -493,11 +493,12 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
     case JS_CONSTRUCT:
       translation->BeginConstructStubFrame(closure_id, translation_size);
       break;
+    case JS_SETTER:
+      // TODO(svenpanne) Implement me!
+      break;
     case ARGUMENTS_ADAPTOR:
       translation->BeginArgumentsAdaptorFrame(closure_id, translation_size);
       break;
-    default:
-      UNREACHABLE();
   }
   for (int i = 0; i < translation_size; ++i) {
     LOperand* value = environment->values()->at(i);

src/hydrogen-instructions.h

@@ -1403,20 +1403,28 @@ class HStackCheck: public HTemplateInstruction<1> {
 };
 
 
+enum InliningKind {
+  NORMAL_RETURN,          // Normal function/method call and return.
+  DROP_EXTRA_ON_RETURN,   // Drop an extra value from the environment on return.
+  CONSTRUCT_CALL_RETURN,  // Either use allocated receiver or return value.
+  SETTER_CALL_RETURN      // Use the RHS of the assignment as the return value.
+};
+
+
 class HEnterInlined: public HTemplateInstruction<0> {
  public:
   HEnterInlined(Handle<JSFunction> closure,
                 int arguments_count,
                 FunctionLiteral* function,
                 CallKind call_kind,
-                bool is_construct,
+                InliningKind inlining_kind,
                 Variable* arguments_var,
                 ZoneList<HValue*>* arguments_values)
       : closure_(closure),
         arguments_count_(arguments_count),
         function_(function),
         call_kind_(call_kind),
-        is_construct_(is_construct),
+        inlining_kind_(inlining_kind),
         arguments_var_(arguments_var),
         arguments_values_(arguments_values) {
   }
@@ -1427,7 +1435,7 @@ class HEnterInlined: public HTemplateInstruction<0> {
   int arguments_count() const { return arguments_count_; }
   FunctionLiteral* function() const { return function_; }
   CallKind call_kind() const { return call_kind_; }
-  bool is_construct() const { return is_construct_; }
+  InliningKind inlining_kind() const { return inlining_kind_; }
 
   virtual Representation RequiredInputRepresentation(int index) {
     return Representation::None();
@@ -1443,7 +1451,7 @@ class HEnterInlined: public HTemplateInstruction<0> {
   int arguments_count_;
   FunctionLiteral* function_;
   CallKind call_kind_;
-  bool is_construct_;
+  InliningKind inlining_kind_;
   Variable* arguments_var_;
   ZoneList<HValue*>* arguments_values_;
 };

src/hydrogen.h

@@ -135,9 +135,7 @@ class HBasicBlock: public ZoneObject {
 
   // Add the inlined function exit sequence, adding an HLeaveInlined
   // instruction and updating the bailout environment.
-  void AddLeaveInlined(HValue* return_value,
-                       HBasicBlock* target,
-                       FunctionState* state = NULL);
+  void AddLeaveInlined(HValue* return_value, FunctionState* state);
 
   // If a target block is tagged as an inline function return, all
   // predecessors should contain the inlined exit sequence:
@@ -396,7 +394,12 @@ Zone* HBasicBlock::zone() const { return graph_->zone(); }
 
 
 // Type of stack frame an environment might refer to.
-enum FrameType { JS_FUNCTION, JS_CONSTRUCT, ARGUMENTS_ADAPTOR };
+enum FrameType {
+  JS_FUNCTION,
+  JS_CONSTRUCT,
+  JS_SETTER,
+  ARGUMENTS_ADAPTOR
+};
 
 
 class HEnvironment: public ZoneObject {
@@ -510,7 +513,7 @@ class HEnvironment: public ZoneObject {
                                 FunctionLiteral* function,
                                 HConstant* undefined,
                                 CallKind call_kind,
-                                bool is_construct) const;
+                                InliningKind inlining_kind) const;
 
   void AddIncomingEdge(HBasicBlock* block, HEnvironment* other);
 
@@ -707,26 +710,18 @@ class TestContext: public AstContext {
 };
 
 
-enum ReturnHandlingFlag {
-  NORMAL_RETURN,
-  DROP_EXTRA_ON_RETURN,
-  CONSTRUCT_CALL_RETURN
-};
-
-
 class FunctionState {
  public:
   FunctionState(HGraphBuilder* owner,
                 CompilationInfo* info,
                 TypeFeedbackOracle* oracle,
-                ReturnHandlingFlag return_handling);
+                InliningKind inlining_kind);
   ~FunctionState();
 
   CompilationInfo* compilation_info() { return compilation_info_; }
   TypeFeedbackOracle* oracle() { return oracle_; }
   AstContext* call_context() { return call_context_; }
-  bool drop_extra() { return return_handling_ == DROP_EXTRA_ON_RETURN; }
-  bool is_construct() { return return_handling_ == CONSTRUCT_CALL_RETURN; }
+  InliningKind inlining_kind() const { return inlining_kind_; }
   HBasicBlock* function_return() { return function_return_; }
   TestContext* test_context() { return test_context_; }
   void ClearInlinedTestContext() {
@@ -756,11 +751,8 @@ class FunctionState {
   // inlined. NULL when not inlining.
   AstContext* call_context_;
 
-  // Indicate whether we have to perform special handling on return from
-  // inlined functions.
-  // - DROP_EXTRA_ON_RETURN: Drop an extra value from the environment.
-  // - CONSTRUCT_CALL_RETURN: Either use allocated receiver or return value.
-  ReturnHandlingFlag return_handling_;
+  // The kind of call which is currently being inlined.
+  InliningKind inlining_kind_;
 
   // When inlining in an effect or value context, this is the return block.
   // It is NULL otherwise.  When inlining in a test context, there are a
@@ -1037,14 +1029,17 @@ class HGraphBuilder: public AstVisitor {
   bool TryInline(CallKind call_kind,
                  Handle<JSFunction> target,
                  int arguments_count,
-                 HValue* receiver,
+                 HValue* implicit_return_value,
                  BailoutId ast_id,
                  BailoutId return_id,
-                 ReturnHandlingFlag return_handling);
+                 InliningKind inlining_kind);
 
   bool TryInlineCall(Call* expr, bool drop_extra = false);
-  bool TryInlineConstruct(CallNew* expr, HValue* receiver);
+  bool TryInlineConstruct(CallNew* expr, HValue* implicit_return_value);
   bool TryInlineGetter(Handle<JSFunction> getter, Property* prop);
+  bool TryInlineSetter(Handle<JSFunction> setter,
+                       Assignment* assignment,
+                       HValue* implicit_return_value);
   bool TryInlineBuiltinMethodCall(Call* expr,
                                   HValue* receiver,
                                   Handle<Map> receiver_map,

src/ia32/lithium-codegen-ia32.cc

@@ -432,11 +432,12 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
     case JS_CONSTRUCT:
       translation->BeginConstructStubFrame(closure_id, translation_size);
       break;
+    case JS_SETTER:
+      // TODO(svenpanne) Implement me!
+      break;
     case ARGUMENTS_ADAPTOR:
       translation->BeginArgumentsAdaptorFrame(closure_id, translation_size);
       break;
-    default:
-      UNREACHABLE();
   }
   for (int i = 0; i < translation_size; ++i) {
     LOperand* value = environment->values()->at(i);

src/ia32/lithium-ia32.cc

@@ -2354,7 +2354,7 @@ LInstruction* LChunkBuilder::DoEnterInlined(HEnterInlined* instr) {
                                                instr->function(),
                                                undefined,
                                                instr->call_kind(),
-                                               instr->is_construct());
+                                               instr->inlining_kind());
   if (instr->arguments_var() != NULL) {
     inner->Bind(instr->arguments_var(), graph()->GetArgumentsObject());
   }

src/mips/lithium-codegen-mips.cc

@@ -461,11 +461,12 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
     case JS_CONSTRUCT:
       translation->BeginConstructStubFrame(closure_id, translation_size);
       break;
+    case JS_SETTER:
+      // TODO(svenpanne) Implement me!
+      break;
     case ARGUMENTS_ADAPTOR:
       translation->BeginArgumentsAdaptorFrame(closure_id, translation_size);
       break;
-    default:
-      UNREACHABLE();
   }
   for (int i = 0; i < translation_size; ++i) {
     LOperand* value = environment->values()->at(i);

src/mips/lithium-mips.cc

@@ -2179,7 +2179,7 @@ LInstruction* LChunkBuilder::DoEnterInlined(HEnterInlined* instr) {
                                                instr->function(),
                                                undefined,
                                                instr->call_kind(),
-                                               instr->is_construct());
+                                               instr->inlining_kind());
   if (instr->arguments_var() != NULL) {
     inner->Bind(instr->arguments_var(), graph()->GetArgumentsObject());
   }

src/x64/lithium-codegen-x64.cc

@@ -380,11 +380,12 @@ void LCodeGen::WriteTranslation(LEnvironment* environment,
     case JS_CONSTRUCT:
       translation->BeginConstructStubFrame(closure_id, translation_size);
       break;
+    case JS_SETTER:
+      // TODO(svenpanne) Implement me!
+      break;
     case ARGUMENTS_ADAPTOR:
       translation->BeginArgumentsAdaptorFrame(closure_id, translation_size);
       break;
-    default:
-      UNREACHABLE();
   }
   for (int i = 0; i < translation_size; ++i) {
     LOperand* value = environment->values()->at(i);

src/x64/lithium-x64.cc

@@ -2238,7 +2238,7 @@ LInstruction* LChunkBuilder::DoEnterInlined(HEnterInlined* instr) {
                                                instr->function(),
                                                undefined,
                                                instr->call_kind(),
-                                               instr->is_construct());
+                                               instr->inlining_kind());
   if (instr->arguments_var() != NULL) {
     inner->Bind(instr->arguments_var(), graph()->GetArgumentsObject());
   }

test/mjsunit/compiler/inline-accessors.js

+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax --inline-accessors
+
+var accessorCallCount, setterArgument, setterValue, obj, forceDeopt;
+
+// -----------------------------------------------------------------------------
+// Helpers for testing inlining of getters.
+
+function TestInlinedGetter(context, expected) {
+  forceDeopt = 0;
+  accessorCallCount = 0;
+
+  assertEquals(expected, context());
+  assertEquals(1, accessorCallCount);
+
+  assertEquals(expected, context());
+  assertEquals(2, accessorCallCount);
+
+  %OptimizeFunctionOnNextCall(context);
+  assertEquals(expected, context());
+  assertEquals(3, accessorCallCount);
+
+  %DeoptimizeFunction(context);
+  %ClearFunctionTypeFeedback(context);
+}
+
+
+function TestGetterInAllContexts(obj, expected) {
+  function value_context() {
+    return obj.getterProperty;
+  }
+  TestInlinedGetter(value_context, expected);
+
+  function test_context() {
+    if (obj.getterProperty) {
+      return 111;
+    } else {
+      return 222;
+    }
+  }
+  TestInlinedGetter(test_context, expected ? 111 : 222);
+
+  function effect_context() {
+    obj.getterProperty;
+    return 5678;
+  }
+  TestInlinedGetter(effect_context, 5678);
+}
+
+// -----------------------------------------------------------------------------
+// Test getter returning something 'true'ish in all contexts.
+
+function getter1() {
+  assertSame(obj, this);
+  accessorCallCount++;
+  forceDeopt + 1;
+  return 1234;
+}
+
+function ConstrG1() { }
+obj = Object.defineProperty(new ConstrG1(), "getterProperty", { get: getter1 });
+TestGetterInAllContexts(obj, 1234);
+obj = Object.create(obj);
+TestGetterInAllContexts(obj, 1234);
+
+// -----------------------------------------------------------------------------
+// Test getter returning false in all contexts.
+
+function getter2() {
+  assertSame(obj, this);
+  accessorCallCount++;
+  forceDeopt + 1;
+  return false;
+}
+
+function ConstrG2() { }
+obj = Object.defineProperty(new ConstrG2(), "getterProperty", { get: getter2 });
+TestGetterInAllContexts(obj, false);
+obj = Object.create(obj);
+TestGetterInAllContexts(obj, false);
+
+// -----------------------------------------------------------------------------
+// Test getter without a return in all contexts.
+
+function getter3() {
+  assertSame(obj, this);
+  accessorCallCount++;
+  forceDeopt + 1;
+}
+
+function ConstrG3() { }
+obj = Object.defineProperty(new ConstrG3(), "getterProperty", { get: getter3 });
+TestGetterInAllContexts(obj, undefined);
+obj = Object.create(obj);
+TestGetterInAllContexts(obj, undefined);
+
+// -----------------------------------------------------------------------------
+// Test getter with too many arguments without a return in all contexts.
+
+function getter4(a) {
+  assertSame(obj, this);
+  assertEquals(undefined, a);
+  accessorCallCount++;
+  forceDeopt + 1;
+}
+
+function ConstrG4() { }
+obj = Object.defineProperty(new ConstrG4(), "getterProperty", { get: getter4 });
+TestGetterInAllContexts(obj, undefined);
+obj = Object.create(obj);
+TestGetterInAllContexts(obj, undefined);
+
+// -----------------------------------------------------------------------------
+// Test getter with too many arguments with a return in all contexts.
+
+function getter5(a) {
+  assertSame(obj, this);
+  assertEquals(undefined, a);
+  accessorCallCount++;
+  forceDeopt + 1;
+  return 9876;
+}
+
+function ConstrG5() { }
+obj = Object.defineProperty(new ConstrG5(), "getterProperty", { get: getter5 });
+TestGetterInAllContexts(obj, 9876);
+obj = Object.create(obj);
+TestGetterInAllContexts(obj, 9876);
+
+// -----------------------------------------------------------------------------
+// Helpers for testing inlining of setters.
+
+function TestInlinedSetter(context, value, expected) {
+  forceDeopt = 0;
+  accessorCallCount = 0;
+  setterArgument = value;
+
+  assertEquals(expected, context(value));
+  assertEquals(value, setterValue);
+  assertEquals(1, accessorCallCount);
+
+  assertEquals(expected, context(value));
+  assertEquals(value, setterValue);
+  assertEquals(2, accessorCallCount);
+
+  %OptimizeFunctionOnNextCall(context);
+  assertEquals(expected, context(value));
+  assertEquals(value, setterValue);
+  assertEquals(3, accessorCallCount);
+
+  %DeoptimizeFunction(context);
+  %ClearFunctionTypeFeedback(context);
+}
+
+function TestSetterInAllContexts(obj) {
+  function value_context(value) {
+    return obj.setterProperty = value;
+  }
+  TestInlinedSetter(value_context, 111, 111);
+
+  function test_context(value) {
+    if (obj.setterProperty = value) {
+      return 333;
+    } else {
+      return 444;
+    }
+  }
+  TestInlinedSetter(test_context, true, 333);
+  TestInlinedSetter(test_context, false, 444);
+
+  function effect_context(value) {
+    obj.setterProperty = value;
+    return 666;
+  }
+  TestInlinedSetter(effect_context, 555, 666);
+}
+
+// -----------------------------------------------------------------------------
+// Test setter without a return in all contexts.
+
+function setter1(value) {
+  assertSame(obj, this);
+  accessorCallCount++;
+  forceDeopt + 1;
+  setterValue = value;
+}
+
+function ConstrS1() { }
+obj = Object.defineProperty(new ConstrS1(), "setterProperty", { set: setter1 });
+TestSetterInAllContexts(obj);
+obj = Object.create(obj);
+TestSetterInAllContexts(obj);
+
+// -----------------------------------------------------------------------------
+// Test setter returning something different than the RHS in all contexts.
+
+function setter2(value) {
+  assertSame(obj, this);
+  accessorCallCount++;
+  forceDeopt + 1;
+  setterValue = value;
+  return 1000000;
+}
+
+function ConstrS2() { }
+obj = Object.defineProperty(new ConstrS2(), "setterProperty", { set: setter2 });
+TestSetterInAllContexts(obj);
+obj = Object.create(obj);
+TestSetterInAllContexts(obj);
+
+// -----------------------------------------------------------------------------
+// Test setter with too few arguments without a return in all contexts.
+
+function setter3() {
+  assertSame(obj, this);
+  accessorCallCount++;
+  forceDeopt + 1;
+  setterValue = setterArgument;
+}
+
+function ConstrS3() { }
+obj = Object.defineProperty(new ConstrS3(), "setterProperty", { set: setter3 });
+TestSetterInAllContexts(obj);
+obj = Object.create(obj);
+TestSetterInAllContexts(obj);
+
+// -----------------------------------------------------------------------------
+// Test setter with too few arguments with a return in all contexts.
+
+function setter4() {
+  assertSame(obj, this);
+  accessorCallCount++;
+  forceDeopt + 1;
+  setterValue = setterArgument;
+  return 2000000;
+}
+
+function ConstrS4() { }
+obj = Object.defineProperty(new ConstrS4(), "setterProperty", { set: setter4 });
+TestSetterInAllContexts(obj);
+obj = Object.create(obj);
+TestSetterInAllContexts(obj);
+
+// -----------------------------------------------------------------------------
+// Test setter with too many arguments without a return in all contexts.
+
+function setter5(value, foo) {
+  assertSame(obj, this);
+  assertEquals(undefined, foo);
+  accessorCallCount++;
+  forceDeopt + 1;
+  setterValue = value;
+}
+
+function ConstrS5() { }
+obj = Object.defineProperty(new ConstrS5(), "setterProperty", { set: setter5 });
+TestSetterInAllContexts(obj);
+obj = Object.create(obj);
+TestSetterInAllContexts(obj);
+
+// -----------------------------------------------------------------------------
+// Test setter with too many arguments with a return in all contexts.
+
+function setter6(value, foo) {
+  assertSame(obj, this);
+  assertEquals(undefined, foo);
+  accessorCallCount++;
+  forceDeopt + 1;
+  setterValue = value;
+  return 3000000;
+}
+
+function ConstrS6() { }
+obj = Object.defineProperty(new ConstrS6(), "setterProperty", { set: setter6 });
+TestSetterInAllContexts(obj);
+obj = Object.create(obj);
+TestSetterInAllContexts(obj);