Commit aed96e7b authored by Mythri's avatar Mythri Committed by Commit Bot

[Turbofan] Simplify handling of hole check bytecodes in bytecode-graph-builder.

ThrowIfHole bytecodes were handled by introducing deopt points to check
for a hole. To avoid deopt loops a hole check protector was used to
generate control flow if there was a deopt due to a hole. However, the
normal control flow version should be as fast as the deopt version
in general. The deopt version could potentially consume less compile time
but it may not be worth the complexity added. Hence simplifying it to
only construct the control flow.

Bug: v8:6383
Change-Id: Icace11f7a6e21e64e1cebd104496e3f559bc85f7
Reviewed-on: https://chromium-review.googlesource.com/525573Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45783}
parent 7c58b68d
......@@ -6,7 +6,6 @@
#include "src/ast/ast.h"
#include "src/ast/scopes.h"
#include "src/compilation-dependencies.h"
#include "src/compiler/access-builder.h"
#include "src/compiler/compiler-source-position-table.h"
#include "src/compiler/linkage.h"
......@@ -441,7 +440,6 @@ BytecodeGraphBuilder::BytecodeGraphBuilder(
Zone* local_zone, Handle<SharedFunctionInfo> shared_info,
Handle<FeedbackVector> feedback_vector, BailoutId osr_ast_id,
JSGraph* jsgraph, CallFrequency invocation_frequency,
CompilationDependencies* dependencies,
SourcePositionTable* source_positions, int inlining_id,
JSTypeHintLowering::Flags flags)
: local_zone_(local_zone),
......@@ -460,7 +458,6 @@ BytecodeGraphBuilder::BytecodeGraphBuilder(
bytecode_analysis_(nullptr),
environment_(nullptr),
osr_ast_id_(osr_ast_id),
dependencies_(dependencies),
merge_environments_(local_zone),
exception_handlers_(local_zone),
current_exception_handler_(0),
......@@ -1679,21 +1676,6 @@ void BytecodeGraphBuilder::VisitReThrow() {
MergeControlToLeaveFunction(control);
}
Node* BytecodeGraphBuilder::TryBuildHoleCheckWithDeopt(
const Operator* hole_check_operator) {
// TODO(6451): Make the hole protector per function.
if (!jsgraph()->isolate()->IsHoleCheckProtectorIntact()) {
return nullptr;
}
dependencies()->AssumePropertyCell(
jsgraph()->isolate()->factory()->hole_check_protector());
PrepareEagerCheckpoint();
Node* accumulator = environment()->LookupAccumulator();
Node* node = NewNode(hole_check_operator, accumulator);
environment()->BindAccumulator(accumulator);
return node;
}
void BytecodeGraphBuilder::BuildHoleCheckAndThrow(
Node* condition, Runtime::FunctionId runtime_id, Node* name) {
Node* accumulator = environment()->LookupAccumulator();
......@@ -1704,7 +1686,7 @@ void BytecodeGraphBuilder::BuildHoleCheckAndThrow(
NewIfTrue();
Node* node;
const Operator* op = javascript()->CallRuntime(runtime_id);
if (runtime_id == Runtime::kThrowReferenceErrorOnHole) {
if (runtime_id == Runtime::kThrowReferenceError) {
DCHECK(name != nullptr);
node = NewNode(op, name);
} else {
......@@ -1721,35 +1703,22 @@ void BytecodeGraphBuilder::BuildHoleCheckAndThrow(
}
void BytecodeGraphBuilder::VisitThrowReferenceErrorIfHole() {
// To avoid deopt loops, we generate regular control flow, if we ever saw
// a throw because of the hole check.
if (!TryBuildHoleCheckWithDeopt(simplified()->CheckNotTaggedHole())) {
// Generate the control flow to avoid deopt loop.
Node* accumulator = environment()->LookupAccumulator();
Node* check_for_hole = NewNode(simplified()->ReferenceEqual(), accumulator,
jsgraph()->TheHoleConstant());
Node* name =
jsgraph()->Constant(bytecode_iterator().GetConstantForIndexOperand(0));
BuildHoleCheckAndThrow(check_for_hole, Runtime::kThrowReferenceErrorOnHole,
name);
}
BuildHoleCheckAndThrow(check_for_hole, Runtime::kThrowReferenceError, name);
}
void BytecodeGraphBuilder::VisitThrowSuperNotCalledIfHole() {
// To avoid deopt loops, we generate regular control flow, if we ever saw
// a throw because of the hole check.
if (!TryBuildHoleCheckWithDeopt(simplified()->CheckNotTaggedHole())) {
Node* accumulator = environment()->LookupAccumulator();
Node* check_for_hole = NewNode(simplified()->ReferenceEqual(), accumulator,
jsgraph()->TheHoleConstant());
BuildHoleCheckAndThrow(check_for_hole, Runtime::kThrowSuperNotCalled);
}
}
void BytecodeGraphBuilder::VisitThrowSuperAlreadyCalledIfNotHole() {
// To avoid deopt loops, we generate regular control flow, if we ever saw
// a throw because of the hole check.
if (!TryBuildHoleCheckWithDeopt(simplified()->CheckTaggedHole())) {
Node* accumulator = environment()->LookupAccumulator();
Node* check_for_hole = NewNode(simplified()->ReferenceEqual(), accumulator,
jsgraph()->TheHoleConstant());
......@@ -1757,7 +1726,6 @@ void BytecodeGraphBuilder::VisitThrowSuperAlreadyCalledIfNotHole() {
NewNode(simplified()->BooleanNot(), check_for_hole);
BuildHoleCheckAndThrow(check_for_not_hole,
Runtime::kThrowSuperAlreadyCalledError);
}
}
void BytecodeGraphBuilder::BuildBinaryOp(const Operator* op) {
......
......@@ -17,9 +17,6 @@
namespace v8 {
namespace internal {
class CompilationDependencies;
namespace compiler {
class Reduction;
......@@ -33,7 +30,6 @@ class BytecodeGraphBuilder {
Zone* local_zone, Handle<SharedFunctionInfo> shared,
Handle<FeedbackVector> feedback_vector, BailoutId osr_ast_id,
JSGraph* jsgraph, CallFrequency invocation_frequency,
CompilationDependencies* dependencies,
SourcePositionTable* source_positions,
int inlining_id = SourcePosition::kNotInlined,
JSTypeHintLowering::Flags flags = JSTypeHintLowering::kNoFlags);
......@@ -202,7 +198,6 @@ class BytecodeGraphBuilder {
Node* value, FeedbackSlot slot);
Node* TryBuildSimplifiedStoreKeyed(const Operator* op, Node* receiver,
Node* key, Node* value, FeedbackSlot slot);
Node* TryBuildHoleCheckWithDeopt(const Operator* deopt_operator);
// Applies the given early reduction onto the current environment.
void ApplyEarlyReduction(Reduction reduction);
......@@ -325,8 +320,6 @@ class BytecodeGraphBuilder {
needs_eager_checkpoint_ = value;
}
CompilationDependencies* dependencies() { return dependencies_; }
#define DECLARE_VISIT_BYTECODE(name, ...) void Visit##name();
BYTECODE_LIST(DECLARE_VISIT_BYTECODE)
#undef DECLARE_VISIT_BYTECODE
......@@ -343,7 +336,6 @@ class BytecodeGraphBuilder {
const BytecodeAnalysis* bytecode_analysis_;
Environment* environment_;
BailoutId osr_ast_id_;
CompilationDependencies* dependencies_;
// Merge environments are snapshots of the environment at points where the
// control flow merges. This models a forward data flow propagation of all
......
......@@ -784,9 +784,6 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
case IrOpcode::kCheckFloat64Hole:
result = LowerCheckFloat64Hole(node, frame_state);
break;
case IrOpcode::kCheckTaggedHole:
result = LowerCheckTaggedHole(node, frame_state);
break;
case IrOpcode::kCheckNotTaggedHole:
result = LowerCheckNotTaggedHole(node, frame_state);
break;
......@@ -2414,13 +2411,6 @@ Node* EffectControlLinearizer::LowerCheckFloat64Hole(Node* node,
return value;
}
Node* EffectControlLinearizer::LowerCheckTaggedHole(Node* node,
Node* frame_state) {
Node* value = node->InputAt(0);
Node* check = __ WordEqual(value, __ TheHoleConstant());
__ DeoptimizeUnless(DeoptimizeReason::kHole, check, frame_state);
return value;
}
Node* EffectControlLinearizer::LowerCheckNotTaggedHole(Node* node,
Node* frame_state) {
......
......@@ -106,7 +106,6 @@ class V8_EXPORT_PRIVATE EffectControlLinearizer {
Node* LowerStringLessThan(Node* node);
Node* LowerStringLessThanOrEqual(Node* node);
Node* LowerCheckFloat64Hole(Node* node, Node* frame_state);
Node* LowerCheckTaggedHole(Node* node, Node* frame_state);
Node* LowerCheckNotTaggedHole(Node* node, Node* frame_state);
Node* LowerConvertTaggedHoleToUndefined(Node* node);
Node* LowerPlainPrimitiveToNumber(Node* node);
......
......@@ -581,8 +581,7 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
}
BytecodeGraphBuilder graph_builder(
parse_info.zone(), shared_info, feedback_vector, BailoutId::None(),
jsgraph(), call.frequency(), info_->dependencies(), source_positions_,
inlining_id, flags);
jsgraph(), call.frequency(), source_positions_, inlining_id, flags);
graph_builder.CreateGraph(false);
// Extract the inlinee start/end nodes.
......
......@@ -224,8 +224,7 @@
V(CheckedTruncateTaggedToWord32) \
V(CheckedTaggedToFloat64) \
V(CheckedTaggedToTaggedSigned) \
V(CheckedTaggedToTaggedPointer) \
V(CheckTaggedHole)
V(CheckedTaggedToTaggedPointer)
#define SIMPLIFIED_COMPARE_BINOP_LIST(V) \
V(NumberEqual) \
......
......@@ -783,8 +783,7 @@ struct GraphBuilderPhase {
temp_zone, data->info()->shared_info(),
handle(data->info()->closure()->feedback_vector()),
data->info()->osr_ast_id(), data->jsgraph(), CallFrequency(1.0f),
data->info()->dependencies(), data->source_positions(),
SourcePosition::kNotInlined, flags);
data->source_positions(), SourcePosition::kNotInlined, flags);
graph_builder.CreateGraph();
} else {
AstGraphBuilderWithPositions graph_builder(
......
......@@ -28,7 +28,6 @@ Reduction RedundancyElimination::Reduce(Node* node) {
case IrOpcode::kCheckSmi:
case IrOpcode::kCheckString:
case IrOpcode::kCheckSeqString:
case IrOpcode::kCheckTaggedHole:
case IrOpcode::kCheckNotTaggedHole:
case IrOpcode::kCheckedFloat64ToInt32:
case IrOpcode::kCheckedInt32Add:
......
......@@ -2783,10 +2783,6 @@ class RepresentationSelector {
}
return;
}
case IrOpcode::kCheckTaggedHole: {
VisitUnop(node, UseInfo::AnyTagged(), MachineRepresentation::kTagged);
return;
}
case IrOpcode::kCheckNotTaggedHole: {
VisitUnop(node, UseInfo::AnyTagged(), MachineRepresentation::kTagged);
return;
......
......@@ -526,7 +526,6 @@ UnicodeEncoding UnicodeEncodingOf(const Operator* op) {
V(CheckString, 1, 1) \
V(CheckSeqString, 1, 1) \
V(CheckSymbol, 1, 1) \
V(CheckTaggedHole, 1, 0) \
V(CheckNotTaggedHole, 1, 1) \
V(CheckedInt32Add, 2, 1) \
V(CheckedInt32Sub, 2, 1) \
......
......@@ -438,7 +438,6 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
const Operator* CheckedTruncateTaggedToWord32(CheckTaggedInputMode);
const Operator* CheckFloat64Hole(CheckFloat64HoleMode);
const Operator* CheckTaggedHole();
const Operator* CheckNotTaggedHole();
const Operator* ConvertTaggedHoleToUndefined();
......
......@@ -76,8 +76,6 @@ Reduction TypedOptimization::Reduce(Node* node) {
switch (node->opcode()) {
case IrOpcode::kCheckHeapObject:
return ReduceCheckHeapObject(node);
case IrOpcode::kCheckTaggedHole:
return ReduceCheckTaggedHole(node);
case IrOpcode::kCheckNotTaggedHole:
return ReduceCheckNotTaggedHole(node);
case IrOpcode::kCheckMaps:
......@@ -134,16 +132,6 @@ Reduction TypedOptimization::ReduceCheckHeapObject(Node* node) {
return NoChange();
}
Reduction TypedOptimization::ReduceCheckTaggedHole(Node* node) {
Node* const input = NodeProperties::GetValueInput(node, 0);
Type* const input_type = NodeProperties::GetType(input);
if (input_type->Is(Type::Hole())) {
ReplaceWithValue(node, input);
return Replace(input);
}
return NoChange();
}
Reduction TypedOptimization::ReduceCheckNotTaggedHole(Node* node) {
Node* const input = NodeProperties::GetValueInput(node, 0);
Type* const input_type = NodeProperties::GetType(input);
......
......@@ -55,7 +55,6 @@ class V8_EXPORT_PRIVATE TypedOptimization final
Reduction ReduceReferenceEqual(Node* node);
Reduction ReduceSelect(Node* node);
Reduction ReduceSpeculativeToNumber(Node* node);
Reduction ReduceCheckTaggedHole(Node* node);
Reduction ReduceCheckNotTaggedHole(Node* node);
CompilationDependencies* dependencies() const { return dependencies_; }
......
......@@ -1226,9 +1226,6 @@ void Verifier::Visitor::Check(Node* node) {
CheckValueInputIs(node, 0, Type::NumberOrHole());
CheckTypeIs(node, Type::NumberOrUndefined());
break;
case IrOpcode::kCheckTaggedHole:
CheckValueInputIs(node, 0, Type::Any());
break;
case IrOpcode::kCheckNotTaggedHole:
CheckValueInputIs(node, 0, Type::Any());
CheckTypeIs(node, Type::NonInternal());
......
......@@ -2871,10 +2871,6 @@ void Heap::CreateInitialObjects() {
cell->set_value(Smi::FromInt(Isolate::kProtectorValid));
set_array_buffer_neutering_protector(*cell);
cell = factory->NewPropertyCell();
cell->set_value(Smi::FromInt(Isolate::kProtectorValid));
set_hole_check_protector(*cell);
set_serialized_templates(empty_fixed_array());
set_serialized_global_proxy_sizes(empty_fixed_array());
......
......@@ -178,7 +178,6 @@ using v8::MemoryPressureLevel;
V(PropertyCell, array_iterator_protector, ArrayIteratorProtector) \
V(PropertyCell, array_buffer_neutering_protector, \
ArrayBufferNeuteringProtector) \
V(PropertyCell, hole_check_protector, HoleCheckProtector) \
/* Special numbers */ \
V(HeapNumber, nan_value, NanValue) \
V(HeapNumber, hole_nan_value, HoleNanValue) \
......@@ -339,7 +338,6 @@ using v8::MemoryPressureLevel;
V(UninitializedValue) \
V(WeakCellMap) \
V(WithContextMap) \
V(HoleCheckProtector) \
PRIVATE_SYMBOL_LIST(V)
#define FIXED_ARRAY_ELEMENTS_WRITE_BARRIER(heap, array, start, length) \
......
......@@ -3147,7 +3147,7 @@ IGNITION_HANDLER(ThrowReferenceErrorIfHole, InterpreterAssembler) {
BIND(&throw_error);
{
Node* name = LoadConstantPoolEntry(BytecodeOperandIdx(0));
CallRuntime(Runtime::kThrowReferenceErrorOnHole, GetContext(), name);
CallRuntime(Runtime::kThrowReferenceError, GetContext(), name);
// We shouldn't ever return from a throw.
Abort(kUnexpectedReturnFromThrow);
}
......
......@@ -153,10 +153,6 @@ bool Isolate::IsArrayIteratorLookupChainIntact() {
return array_iterator_cell->value() == Smi::FromInt(kProtectorValid);
}
bool Isolate::IsHoleCheckProtectorIntact() {
PropertyCell* hole_check_cell = heap()->hole_check_protector();
return hole_check_cell->value() == Smi::FromInt(kProtectorValid);
}
} // namespace internal
} // namespace v8
......
......@@ -3219,15 +3219,6 @@ void Isolate::InvalidateArrayBufferNeuteringProtector() {
DCHECK(!IsArrayBufferNeuteringIntact());
}
void Isolate::InvalidateHoleCheckProtector() {
DCHECK(factory()->hole_check_protector()->value()->IsSmi());
DCHECK(IsHoleCheckProtectorIntact());
PropertyCell::SetValueWithInvalidation(
factory()->hole_check_protector(),
handle(Smi::FromInt(kProtectorInvalid), this));
DCHECK(!IsHoleCheckProtectorIntact());
}
bool Isolate::IsAnyInitialArrayPrototype(Handle<JSArray> array) {
DisallowHeapAllocation no_gc;
return IsInAnyContext(*array, Context::INITIAL_ARRAY_PROTOTYPE_INDEX);
......
......@@ -1048,7 +1048,6 @@ class Isolate {
bool IsIsConcatSpreadableLookupChainIntact(JSReceiver* receiver);
inline bool IsStringLengthOverflowIntact();
inline bool IsArrayIteratorLookupChainIntact();
inline bool IsHoleCheckProtectorIntact();
// Avoid deopt loops if fast Array Iterators migrate to slow Array Iterators.
inline bool IsFastArrayIterationIntact();
......@@ -1075,7 +1074,6 @@ class Isolate {
void InvalidateStringLengthOverflowProtector();
void InvalidateArrayIteratorProtector();
void InvalidateArrayBufferNeuteringProtector();
void InvalidateHoleCheckProtector();
// Returns true if array is the initial array prototype in any native context.
bool IsAnyInitialArrayPrototype(Handle<JSArray> array);
......
......@@ -48,10 +48,6 @@ RUNTIME_FUNCTION(Runtime_ThrowStaticPrototypeError) {
RUNTIME_FUNCTION(Runtime_ThrowSuperAlreadyCalledError) {
HandleScope scope(isolate);
DCHECK_EQ(0, args.length());
// Invalidate the hole check protector.
if (isolate->IsHoleCheckProtectorIntact()) {
isolate->InvalidateHoleCheckProtector();
}
THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewReferenceError(MessageTemplate::kSuperAlreadyCalled));
}
......@@ -59,10 +55,6 @@ RUNTIME_FUNCTION(Runtime_ThrowSuperAlreadyCalledError) {
RUNTIME_FUNCTION(Runtime_ThrowSuperNotCalled) {
HandleScope scope(isolate);
DCHECK_EQ(0, args.length());
// Invalidate the hole check protector.
if (isolate->IsHoleCheckProtectorIntact()) {
isolate->InvalidateHoleCheckProtector();
}
THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewReferenceError(MessageTemplate::kSuperNotCalled));
}
......
......@@ -303,18 +303,6 @@ RUNTIME_FUNCTION(Runtime_ThrowApplyNonFunction) {
isolate, NewTypeError(MessageTemplate::kApplyNonFunction, object, type));
}
RUNTIME_FUNCTION(Runtime_ThrowReferenceErrorOnHole) {
HandleScope scope(isolate);
DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
// Invalidate the protector.
if (isolate->IsHoleCheckProtectorIntact()) {
isolate->InvalidateHoleCheckProtector();
}
THROW_NEW_ERROR_RETURN_FAILURE(
isolate, NewReferenceError(MessageTemplate::kNotDefined, name));
}
RUNTIME_FUNCTION(Runtime_StackGuard) {
SealHandleScope shs(isolate);
DCHECK_EQ(0, args.length());
......
......@@ -340,7 +340,6 @@ namespace internal {
F(ThrowNotConstructor, 1, 1) \
F(ThrowRangeError, -1 /* >= 1 */, 1) \
F(ThrowReferenceError, 1, 1) \
F(ThrowReferenceErrorOnHole, 1, 1) \
F(ThrowStackOverflow, 0, 1) \
F(ThrowSymbolAsyncIteratorInvalid, 0, 1) \
F(ThrowTypeError, -1 /* >= 1 */, 1) \
......
......@@ -17,11 +17,4 @@ f(0);
assertOptimized(f);
// Check that hole checks are handled correctly in optimized code.
assertThrowsEquals(() => {f(1)}, ReferenceError());
// The first time we introduce a deopt point so on hole f should deopt.
assertUnoptimized(f);
assertTrue(%GetDeoptCount(f) > 0);
%OptimizeFunctionOnNextCall(f);
f(0);
assertThrowsEquals(() => {f(1)}, ReferenceError());
// The second time it should generate normal control flow and not deopt.
assertOptimized(f);
......@@ -23,12 +23,4 @@ test = new B(0);
assertOptimized(B);
// Check that hole checks are handled correctly in optimized code.
assertThrowsEquals(() => {new B(1)}, ReferenceError());
// First time it should Deopt.
assertUnoptimized(B);
assertTrue(%GetDeoptCount(B) > 0);
%OptimizeFunctionOnNextCall(B);
test = new B(0);
assertThrowsEquals(() => {new B(1)}, ReferenceError());
// Second time it should generate normal control flow, to avoid
// deopt loop.
assertOptimized(B);
......@@ -22,12 +22,4 @@ test = new B(1);
assertOptimized(B);
// Check that hole checks are handled correctly in optimized code.
assertThrowsEquals(() => {new B(0)}, ReferenceError());
// First time it should Deopt.
assertUnoptimized(B);
assertTrue(%GetDeoptCount(B) > 0);
%OptimizeFunctionOnNextCall(B);
test = new B(1);
assertThrowsEquals(() => {new B(0)}, ReferenceError());
// Second time it should generate normal control flow, to avoid
// deopt loop.
assertOptimized(B);
......@@ -303,10 +303,9 @@ KNOWN_OBJECTS = {
("OLD_SPACE", 0x02929): "FastArrayIterationProtector",
("OLD_SPACE", 0x02939): "ArrayIteratorProtector",
("OLD_SPACE", 0x02959): "ArrayBufferNeuteringProtector",
("OLD_SPACE", 0x02979): "HoleCheckProtector",
("OLD_SPACE", 0x02999): "InfinityValue",
("OLD_SPACE", 0x029a9): "MinusZeroValue",
("OLD_SPACE", 0x029b9): "MinusInfinityValue",
("OLD_SPACE", 0x02979): "InfinityValue",
("OLD_SPACE", 0x02989): "MinusZeroValue",
("OLD_SPACE", 0x02999): "MinusInfinityValue",
}
# List of known V8 Frame Markers.
......
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