Commit 9750708e authored by bmeurer's avatar bmeurer Committed by Commit bot

[turbofan] Remove untested no-deoptimization code path from JSGlobalObjectSpecialization.

We don't have proper test coverage for the no-deoptimization code paths
in the JSGlobalObjectSpecialization reducer, and we will properly never
have any use for that code, so it just adds complexity and code that
likely breaks over time (as its untested).

R=jarin@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#33680}
parent 7991c226
......@@ -27,11 +27,10 @@ struct JSGlobalObjectSpecialization::ScriptContextTableLookupResult {
JSGlobalObjectSpecialization::JSGlobalObjectSpecialization(
Editor* editor, JSGraph* jsgraph, Flags flags,
Editor* editor, JSGraph* jsgraph,
MaybeHandle<Context> native_context, CompilationDependencies* dependencies)
: AdvancedReducer(editor),
jsgraph_(jsgraph),
flags_(flags),
native_context_(native_context),
dependencies_(dependencies),
type_cache_(TypeCache::Get()) {}
......@@ -49,7 +48,6 @@ Reduction JSGlobalObjectSpecialization::Reduce(Node* node) {
return NoChange();
}
Reduction JSGlobalObjectSpecialization::ReduceJSLoadGlobal(Node* node) {
DCHECK_EQ(IrOpcode::kJSLoadGlobal, node->opcode());
Handle<Name> name = LoadGlobalParametersOf(node->op()).name();
......@@ -88,47 +86,36 @@ Reduction JSGlobalObjectSpecialization::ReduceJSLoadGlobal(Node* node) {
return Replace(value);
}
// Load from non-configurable, data property on the global can be lowered to
// a field load, even without deoptimization, because the property cannot be
// deleted or reconfigured to an accessor/interceptor property. Yet, if
// deoptimization support is available, we can constant-fold certain global
// properties or at least lower them to field loads annotated with more
// precise type feedback.
Type* property_cell_value_type = Type::Tagged();
if (flags() & kDeoptimizationEnabled) {
// Record a code dependency on the cell if we can benefit from the
// additional feedback, or the global property is configurable (i.e.
// can be deleted or reconfigured to an accessor property).
if (property_details.cell_type() != PropertyCellType::kMutable ||
property_details.IsConfigurable()) {
dependencies()->AssumePropertyCell(property_cell);
}
// Record a code dependency on the cell if we can benefit from the
// additional feedback, or the global property is configurable (i.e.
// can be deleted or reconfigured to an accessor property).
if (property_details.cell_type() != PropertyCellType::kMutable ||
property_details.IsConfigurable()) {
dependencies()->AssumePropertyCell(property_cell);
}
// Load from constant/undefined global property can be constant-folded.
if ((property_details.cell_type() == PropertyCellType::kConstant ||
property_details.cell_type() == PropertyCellType::kUndefined)) {
Node* value = jsgraph()->Constant(property_cell_value);
ReplaceWithValue(node, value);
return Replace(value);
}
// Load from constant/undefined global property can be constant-folded.
if (property_details.cell_type() == PropertyCellType::kConstant ||
property_details.cell_type() == PropertyCellType::kUndefined) {
Node* value = jsgraph()->Constant(property_cell_value);
ReplaceWithValue(node, value);
return Replace(value);
}
// Load from constant type cell can benefit from type feedback.
if (property_details.cell_type() == PropertyCellType::kConstantType) {
// Compute proper type based on the current value in the cell.
if (property_cell_value->IsSmi()) {
property_cell_value_type = type_cache_.kSmi;
} else if (property_cell_value->IsNumber()) {
property_cell_value_type = type_cache_.kHeapNumber;
} else {
Handle<Map> property_cell_value_map(
Handle<HeapObject>::cast(property_cell_value)->map(), isolate());
property_cell_value_type =
Type::Class(property_cell_value_map, graph()->zone());
}
// Load from constant type cell can benefit from type feedback.
Type* property_cell_value_type = Type::Tagged();
if (property_details.cell_type() == PropertyCellType::kConstantType) {
// Compute proper type based on the current value in the cell.
if (property_cell_value->IsSmi()) {
property_cell_value_type = type_cache_.kSmi;
} else if (property_cell_value->IsNumber()) {
property_cell_value_type = type_cache_.kHeapNumber;
} else {
Handle<Map> property_cell_value_map(
Handle<HeapObject>::cast(property_cell_value)->map(), isolate());
property_cell_value_type =
Type::Class(property_cell_value_map, graph()->zone());
}
} else if (property_details.IsConfigurable()) {
// Access to configurable global properties requires deoptimization support.
return NoChange();
}
Node* value = effect = graph()->NewNode(
simplified()->LoadField(
......@@ -178,9 +165,8 @@ Reduction JSGlobalObjectSpecialization::ReduceJSStoreGlobal(Node* node) {
return NoChange();
}
case PropertyCellType::kConstant: {
// Store to constant property cell requires deoptimization support,
// because we might even need to eager deoptimize for mismatch.
if (!(flags() & kDeoptimizationEnabled)) return NoChange();
// Record a code dependency on the cell, and just deoptimize if the new
// value doesn't match the previous value stored inside the cell.
dependencies()->AssumePropertyCell(property_cell);
Node* check =
graph()->NewNode(simplified()->ReferenceEqual(Type::Tagged()), value,
......@@ -197,9 +183,8 @@ Reduction JSGlobalObjectSpecialization::ReduceJSStoreGlobal(Node* node) {
break;
}
case PropertyCellType::kConstantType: {
// Store to constant-type property cell requires deoptimization support,
// because we might even need to eager deoptimize for mismatch.
if (!(flags() & kDeoptimizationEnabled)) return NoChange();
// Record a code dependency on the cell, and just deoptimize if the new
// values' type doesn't match the type of the previous value in the cell.
dependencies()->AssumePropertyCell(property_cell);
Node* check = graph()->NewNode(simplified()->ObjectIsSmi(), value);
Type* property_cell_value_type = Type::TaggedSigned();
......@@ -243,13 +228,11 @@ Reduction JSGlobalObjectSpecialization::ReduceJSStoreGlobal(Node* node) {
}
case PropertyCellType::kMutable: {
// Store to non-configurable, data property on the global can be lowered
// to a field store, even without deoptimization, because the property
// cannot be deleted or reconfigured to an accessor/interceptor property.
// to a field store, even without recording a code dependency on the cell,
// because the property cannot be deleted or reconfigured to an accessor
// or interceptor property.
if (property_details.IsConfigurable()) {
// With deoptimization support, we can lower stores even to configurable
// data properties on the global object, by adding a code dependency on
// the cell.
if (!(flags() & kDeoptimizationEnabled)) return NoChange();
// Protect lowering by recording a code dependency on the cell.
dependencies()->AssumePropertyCell(property_cell);
}
effect = graph()->NewNode(
......
......@@ -5,7 +5,6 @@
#ifndef V8_COMPILER_JS_GLOBAL_OBJECT_SPECIALIZATION_H_
#define V8_COMPILER_JS_GLOBAL_OBJECT_SPECIALIZATION_H_
#include "src/base/flags.h"
#include "src/compiler/graph-reducer.h"
namespace v8 {
......@@ -30,14 +29,7 @@ class SimplifiedOperatorBuilder;
// nodes.
class JSGlobalObjectSpecialization final : public AdvancedReducer {
public:
// Flags that control the mode of operation.
enum Flag {
kNoFlags = 0u,
kDeoptimizationEnabled = 1u << 0,
};
typedef base::Flags<Flag> Flags;
JSGlobalObjectSpecialization(Editor* editor, JSGraph* jsgraph, Flags flags,
JSGlobalObjectSpecialization(Editor* editor, JSGraph* jsgraph,
MaybeHandle<Context> native_context,
CompilationDependencies* dependencies);
......@@ -61,12 +53,10 @@ class JSGlobalObjectSpecialization final : public AdvancedReducer {
CommonOperatorBuilder* common() const;
JSOperatorBuilder* javascript() const;
SimplifiedOperatorBuilder* simplified() const;
Flags flags() const { return flags_; }
MaybeHandle<Context> native_context() const { return native_context_; }
CompilationDependencies* dependencies() const { return dependencies_; }
JSGraph* const jsgraph_;
Flags const flags_;
MaybeHandle<Context> native_context_;
CompilationDependencies* const dependencies_;
TypeCache const& type_cache_;
......@@ -74,8 +64,6 @@ class JSGlobalObjectSpecialization final : public AdvancedReducer {
DISALLOW_COPY_AND_ASSIGN(JSGlobalObjectSpecialization);
};
DEFINE_OPERATORS_FOR_FLAGS(JSGlobalObjectSpecialization::Flags)
} // namespace compiler
} // namespace internal
} // namespace v8
......
......@@ -552,11 +552,8 @@ struct InliningPhase {
JSFrameSpecialization frame_specialization(data->info()->osr_frame(),
data->jsgraph());
JSGlobalObjectSpecialization global_object_specialization(
&graph_reducer, data->jsgraph(),
data->info()->is_deoptimization_enabled()
? JSGlobalObjectSpecialization::kDeoptimizationEnabled
: JSGlobalObjectSpecialization::kNoFlags,
data->native_context(), data->info()->dependencies());
&graph_reducer, data->jsgraph(), data->native_context(),
data->info()->dependencies());
JSNativeContextSpecialization native_context_specialization(
&graph_reducer, data->jsgraph(),
data->info()->is_deoptimization_enabled()
......@@ -573,7 +570,9 @@ struct InliningPhase {
if (data->info()->is_frame_specializing()) {
AddReducer(data, &graph_reducer, &frame_specialization);
}
AddReducer(data, &graph_reducer, &global_object_specialization);
if (data->info()->is_deoptimization_enabled()) {
AddReducer(data, &graph_reducer, &global_object_specialization);
}
AddReducer(data, &graph_reducer, &native_context_specialization);
AddReducer(data, &graph_reducer, &context_specialization);
AddReducer(data, &graph_reducer, &call_reducer);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment