Revert of [turbofan] Optimize string "length" property access based on types....

Revert of [turbofan] Optimize string "length" property access based on types. (patchset #2 id:20001 of https://codereview.chromium.org/1216593003/)

Reason for revert:
Causes crash when running benchmarks/octane/regexp.js on ARM:
http://build.chromium.org/p/client.v8/builders/V8%20Arm/builds/2492/steps/Benchmarks/logs/regexp

Original issue's description:
> [turbofan] Optimize string "length" property access based on types.
>
> Optimize string "length" property access based on static type
> information if possible, but also optimistically optimize the access
> based on type feedback from the LoadIC.
>
> R=jarin@chromium.org
>
> Committed: https://crrev.com/17add22ff4b9c5ca638502e7708f0d9d99baca40
> Cr-Commit-Position: refs/heads/master@{#29543}

TBR=mstarzinger@chromium.org,jarin@chromium.org,bmeurer@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1224083002

Cr-Commit-Position: refs/heads/master@{#29544}

BUILD.gn

@@ -686,8 +686,6 @@ source_set("v8_base") {
     "src/compiler/js-operator.h",
     "src/compiler/js-type-feedback.cc",
     "src/compiler/js-type-feedback.h",
-    "src/compiler/js-type-feedback-lowering.cc",
-    "src/compiler/js-type-feedback-lowering.h",
     "src/compiler/js-typed-lowering.cc",
     "src/compiler/js-typed-lowering.h",
     "src/compiler/jump-threading.cc",

src/compiler/access-builder.cc

@@ -115,9 +115,7 @@ FieldAccess AccessBuilder::ForMapInstanceType() {
 // static
 FieldAccess AccessBuilder::ForStringLength(Zone* zone) {
   return {kTaggedBase, String::kLengthOffset, Handle<Name>(),
-          Type::Intersect(Type::Range(0, String::kMaxLength, zone),
-                          Type::TaggedSigned(), zone),
-          kMachAnyTagged};
+          Type::Range(0, String::kMaxLength, zone), kMachAnyTagged};
 }
 
 

src/compiler/js-type-feedback-lowering.cc

-// Copyright 2015 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "src/compiler/js-type-feedback-lowering.h"
-
-#include "src/compiler/access-builder.h"
-#include "src/compiler/js-graph.h"
-#include "src/compiler/node-properties.h"
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-JSTypeFeedbackLowering::JSTypeFeedbackLowering(Editor* editor, Flags flags,
-                                               JSGraph* jsgraph)
-    : AdvancedReducer(editor),
-      flags_(flags),
-      jsgraph_(jsgraph),
-      simplified_(graph()->zone()) {}
-
-
-Reduction JSTypeFeedbackLowering::Reduce(Node* node) {
-  switch (node->opcode()) {
-    case IrOpcode::kJSLoadNamed:
-      return ReduceJSLoadNamed(node);
-    default:
-      break;
-  }
-  return NoChange();
-}
-
-
-Reduction JSTypeFeedbackLowering::ReduceJSLoadNamed(Node* node) {
-  DCHECK_EQ(IrOpcode::kJSLoadNamed, node->opcode());
-  Node* receiver = NodeProperties::GetValueInput(node, 0);
-  Type* receiver_type = NodeProperties::GetBounds(receiver).upper;
-  Node* frame_state = NodeProperties::GetFrameStateInput(node, 1);
-  Node* effect = NodeProperties::GetEffectInput(node);
-  Node* control = NodeProperties::GetControlInput(node);
-  // We need to make optimistic assumptions to continue.
-  if (!(flags() & kDeoptimizationEnabled)) return NoChange();
-  LoadNamedParameters const& p = LoadNamedParametersOf(node->op());
-  Handle<TypeFeedbackVector> vector;
-  if (!p.feedback().vector().ToHandle(&vector)) return NoChange();
-  if (p.name().handle().is_identical_to(factory()->length_string())) {
-    LoadICNexus nexus(vector, p.feedback().slot());
-    MapHandleList maps;
-    if (nexus.ExtractMaps(&maps) > 0) {
-      for (Handle<Map> map : maps) {
-        if (map->instance_type() >= FIRST_NONSTRING_TYPE) return NoChange();
-      }
-      // Optimistic optimization for "length" property of strings.
-      if (receiver_type->Maybe(Type::TaggedSigned())) {
-        Node* check = graph()->NewNode(simplified()->ObjectIsSmi(), receiver);
-        Node* branch = graph()->NewNode(common()->Branch(BranchHint::kFalse),
-                                        check, control);
-        Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
-        Node* deoptimize = graph()->NewNode(common()->Deoptimize(), frame_state,
-                                            effect, if_true);
-        // TODO(bmeurer): This should be on the AdvancedReducer somehow.
-        NodeProperties::MergeControlToEnd(graph(), common(), deoptimize);
-        control = graph()->NewNode(common()->IfFalse(), branch);
-      }
-      Node* receiver_map = effect =
-          graph()->NewNode(simplified()->LoadField(AccessBuilder::ForMap()),
-                           receiver, effect, control);
-      Node* receiver_instance_type = effect = graph()->NewNode(
-          simplified()->LoadField(AccessBuilder::ForMapInstanceType()),
-          receiver_map, effect, control);
-      Node* check =
-          graph()->NewNode(machine()->Uint32LessThan(), receiver_instance_type,
-                           jsgraph()->Uint32Constant(FIRST_NONSTRING_TYPE));
-      Node* branch =
-          graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
-      Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
-      Node* deoptimize = graph()->NewNode(common()->Deoptimize(), frame_state,
-                                          effect, if_false);
-      // TODO(bmeurer): This should be on the AdvancedReducer somehow.
-      NodeProperties::MergeControlToEnd(graph(), common(), deoptimize);
-      control = graph()->NewNode(common()->IfTrue(), branch);
-      Node* value = effect =
-          graph()->NewNode(simplified()->LoadField(
-                               AccessBuilder::ForStringLength(graph()->zone())),
-                           receiver, effect, control);
-      ReplaceWithValue(node, value, effect, control);
-      return Replace(value);
-    }
-  }
-  return NoChange();
-}
-
-
-Factory* JSTypeFeedbackLowering::factory() const {
-  return isolate()->factory();
-}
-
-
-CommonOperatorBuilder* JSTypeFeedbackLowering::common() const {
-  return jsgraph()->common();
-}
-
-
-Graph* JSTypeFeedbackLowering::graph() const { return jsgraph()->graph(); }
-
-
-Isolate* JSTypeFeedbackLowering::isolate() const {
-  return jsgraph()->isolate();
-}
-
-
-MachineOperatorBuilder* JSTypeFeedbackLowering::machine() const {
-  return jsgraph()->machine();
-}
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8

src/compiler/js-type-feedback-lowering.h

-// Copyright 2015 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef V8_COMPILER_JS_TYPE_FEEDBACK_LOWERING_H_
-#define V8_COMPILER_JS_TYPE_FEEDBACK_LOWERING_H_
-
-#include "src/base/flags.h"
-#include "src/compiler/graph-reducer.h"
-#include "src/compiler/simplified-operator.h"
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-// Forward declarations.
-class CommonOperatorBuilder;
-class JSGraph;
-class MachineOperatorBuilder;
-
-
-// Lowers JS-level operators to simplified operators based on type feedback.
-class JSTypeFeedbackLowering final : public AdvancedReducer {
- public:
-  // Various configuration flags to control the operation of this lowering.
-  enum Flag {
-    kNoFlags = 0,
-    kDeoptimizationEnabled = 1 << 0,
-  };
-  typedef base::Flags<Flag> Flags;
-
-  JSTypeFeedbackLowering(Editor* editor, Flags flags, JSGraph* jsgraph);
-  ~JSTypeFeedbackLowering() final {}
-
-  Reduction Reduce(Node* node) final;
-
- private:
-  Reduction ReduceJSLoadNamed(Node* node);
-
-  Factory* factory() const;
-  Flags flags() const { return flags_; }
-  Graph* graph() const;
-  Isolate* isolate() const;
-  JSGraph* jsgraph() const { return jsgraph_; }
-  CommonOperatorBuilder* common() const;
-  MachineOperatorBuilder* machine() const;
-  SimplifiedOperatorBuilder* simplified() { return &simplified_; }
-
-  Flags const flags_;
-  JSGraph* const jsgraph_;
-  SimplifiedOperatorBuilder simplified_;
-
-  DISALLOW_COPY_AND_ASSIGN(JSTypeFeedbackLowering);
-};
-
-DEFINE_OPERATORS_FOR_FLAGS(JSTypeFeedbackLowering::Flags)
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_COMPILER_JS_TYPE_FEEDBACK_LOWERING_H_

src/compiler/js-typed-lowering.cc

@@ -797,27 +797,6 @@ Reduction JSTypedLowering::ReduceJSLoadGlobal(Node* node) {
 }
 
 
-Reduction JSTypedLowering::ReduceJSLoadNamed(Node* node) {
-  DCHECK_EQ(IrOpcode::kJSLoadNamed, node->opcode());
-  Node* receiver = NodeProperties::GetValueInput(node, 0);
-  Type* receiver_type = NodeProperties::GetBounds(receiver).upper;
-  Node* effect = NodeProperties::GetEffectInput(node);
-  Node* control = NodeProperties::GetControlInput(node);
-  Handle<Name> name = LoadNamedParametersOf(node->op()).name().handle();
-  // Optimize "length" property of strings.
-  if (name.is_identical_to(factory()->length_string()) &&
-      receiver_type->Is(Type::String())) {
-    Node* value = effect =
-        graph()->NewNode(simplified()->LoadField(
-                             AccessBuilder::ForStringLength(graph()->zone())),
-                         receiver, effect, control);
-    ReplaceWithValue(node, value, effect);
-    return Replace(value);
-  }
-  return NoChange();
-}
-
-
 Reduction JSTypedLowering::ReduceJSLoadProperty(Node* node) {
   Node* key = NodeProperties::GetValueInput(node, 1);
   Node* base = NodeProperties::GetValueInput(node, 0);
@@ -1642,8 +1621,6 @@ Reduction JSTypedLowering::Reduce(Node* node) {
       return ReduceJSToString(node);
     case IrOpcode::kJSLoadGlobal:
       return ReduceJSLoadGlobal(node);
-    case IrOpcode::kJSLoadNamed:
-      return ReduceJSLoadNamed(node);
     case IrOpcode::kJSLoadProperty:
       return ReduceJSLoadProperty(node);
     case IrOpcode::kJSStoreProperty:

src/compiler/js-typed-lowering.h

@@ -42,7 +42,6 @@ class JSTypedLowering final : public AdvancedReducer {
   Reduction ReduceJSMultiply(Node* node);
   Reduction ReduceJSComparison(Node* node);
   Reduction ReduceJSLoadGlobal(Node* node);
-  Reduction ReduceJSLoadNamed(Node* node);
   Reduction ReduceJSLoadProperty(Node* node);
   Reduction ReduceJSStoreProperty(Node* node);
   Reduction ReduceJSLoadContext(Node* node);

src/compiler/pipeline.cc

@@ -31,7 +31,6 @@
 #include "src/compiler/js-inlining.h"
 #include "src/compiler/js-intrinsic-lowering.h"
 #include "src/compiler/js-type-feedback.h"
-#include "src/compiler/js-type-feedback-lowering.h"
 #include "src/compiler/js-typed-lowering.h"
 #include "src/compiler/jump-threading.h"
 #include "src/compiler/load-elimination.h"
@@ -578,11 +577,6 @@ struct TypedLoweringPhase {
     LoadElimination load_elimination(&graph_reducer);
     JSBuiltinReducer builtin_reducer(&graph_reducer, data->jsgraph());
     JSTypedLowering typed_lowering(&graph_reducer, data->jsgraph(), temp_zone);
-    JSTypeFeedbackLowering type_feedback_lowering(
-        &graph_reducer, data->info()->is_deoptimization_enabled()
-                            ? JSTypeFeedbackLowering::kDeoptimizationEnabled
-                            : JSTypeFeedbackLowering::kNoFlags,
-        data->jsgraph());
     JSIntrinsicLowering intrinsic_lowering(
         &graph_reducer, data->jsgraph(),
         data->info()->is_deoptimization_enabled()
@@ -594,7 +588,6 @@ struct TypedLoweringPhase {
     AddReducer(data, &graph_reducer, &builtin_reducer);
     AddReducer(data, &graph_reducer, &typed_lowering);
     AddReducer(data, &graph_reducer, &intrinsic_lowering);
-    AddReducer(data, &graph_reducer, &type_feedback_lowering);
     AddReducer(data, &graph_reducer, &load_elimination);
     AddReducer(data, &graph_reducer, &common_reducer);
     graph_reducer.ReduceGraph();

test/mjsunit/compiler/string-length.js

-// Copyright 2015 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// Flags: --allow-natives-syntax
-
-assertEquals(0, "".length);
-assertEquals(1, "a".length);
-assertEquals(2, ("a" + "b").length);
-
-function id(x) { return x; }
-
-function f1(x) {
-  return x.length;
-}
-assertEquals(0, f1(""));
-assertEquals(1, f1("a"));
-%OptimizeFunctionOnNextCall(f1);
-assertEquals(2, f1("a" + "b"));
-assertEquals(3, f1(id("a") + id("b" + id("c"))))
-
-function f2(x, y, z) {
-  x = x ? "" + y : "" + z;
-  return x.length;
-}
-assertEquals(0, f2(true, "", "a"));
-assertEquals(1, f2(false, "", "a"));
-%OptimizeFunctionOnNextCall(f2);
-assertEquals(0, f2(true, "", "a"));
-assertEquals(1, f2(false, "", "a"));
-assertEquals(3, f2(true, id("a") + id("b" + id("c")), ""));

test/unittests/compiler/js-typed-lowering-unittest.cc

@@ -874,11 +874,7 @@ TEST_F(JSTypedLoweringTest, JSStorePropertyToExternalTypedArrayWithSafeKey) {
 }
 
 
-// -----------------------------------------------------------------------------
-// JSLoadGlobal
-
-
-TEST_F(JSTypedLoweringTest, JSLoadGlobalConstants) {
+TEST_F(JSTypedLoweringTest, JSLoadNamedGlobalConstants) {
   Handle<String> names[] = {
       Handle<String>(isolate()->heap()->undefined_string(), isolate()),
       Handle<String>(isolate()->heap()->infinity_string(), isolate()),
@@ -910,31 +906,6 @@ TEST_F(JSTypedLoweringTest, JSLoadGlobalConstants) {
 }
 
 
-// -----------------------------------------------------------------------------
-// JSLoadNamed
-
-
-TEST_F(JSTypedLoweringTest, JSLoadNamedStringLength) {
-  VectorSlotPair feedback;
-  Unique<Name> name = Unique<Name>::CreateImmovable(factory()->length_string());
-  Node* const receiver = Parameter(Type::String(), 0);
-  Node* const vector = Parameter(Type::Internal(), 1);
-  Node* const context = UndefinedConstant();
-  Node* const effect = graph()->start();
-  Node* const control = graph()->start();
-  TRACED_FOREACH(LanguageMode, language_mode, kLanguageModes) {
-    Reduction const r = Reduce(
-        graph()->NewNode(javascript()->LoadNamed(name, feedback, language_mode),
-                         receiver, vector, context, EmptyFrameState(),
-                         EmptyFrameState(), effect, control));
-    ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(),
-                IsLoadField(AccessBuilder::ForStringLength(zone()), receiver,
-                            effect, control));
-  }
-}
-
-
 // -----------------------------------------------------------------------------
 // JSLoadDynamicGlobal
 

tools/gyp/v8.gyp

@@ -519,8 +519,6 @@
         '../../src/compiler/js-operator.h',
         '../../src/compiler/js-type-feedback.cc',
         '../../src/compiler/js-type-feedback.h',
-        '../../src/compiler/js-type-feedback-lowering.cc',
-        '../../src/compiler/js-type-feedback-lowering.h',
         '../../src/compiler/js-typed-lowering.cc',
         '../../src/compiler/js-typed-lowering.h',
         '../../src/compiler/jump-threading.cc',