v8/test/cctest/compiler/test-js-context-specialization.cc

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

705 lines
25 KiB
C++
Raw Normal View History

// Copyright 2014 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/codegen/tick-counter.h"
This CL enables precise source positions for all V8 compilers. It merges compiler::SourcePosition and internal::SourcePosition to a single class used throughout the codebase. The new internal::SourcePosition instances store an id identifying an inlined function in addition to a script offset. SourcePosition::InliningId() refers to a the new table DeoptimizationInputData::InliningPositions(), which provides the following data for every inlining id: - The inlined SharedFunctionInfo as an offset into DeoptimizationInfo::LiteralArray - The SourcePosition of the inlining. Recursively, this yields the full inlining stack. Before the Code object is created, the same information can be found in CompilationInfo::inlined_functions(). If SourcePosition::InliningId() is SourcePosition::kNotInlined, it refers to the outer (non-inlined) function. So every SourcePosition has full information about its inlining stack, as long as the corresponding Code object is known. The internal represenation of a source position is a positive 64bit integer. All compilers create now appropriate source positions for inlined functions. In the case of Turbofan, this required using AstGraphBuilderWithPositions for inlined functions too. So this class is now moved to a header file. At the moment, the additional information in source positions is only used in --trace-deopt and --code-comments. The profiler needs to be updated, at the moment it gets the correct script offsets from the deopt info, but the wrong script id from the reconstructed deopt stack, which can lead to wrong outputs. This should be resolved by making the profiler use the new inlining information for deopts. I activated the inlined deoptimization tests in test-cpu-profiler.cc for Turbofan, changing them to a case where the deopt stack and the inlining position agree. It is currently still broken for other cases. The following additional changes were necessary: - The source position table (internal::SourcePositionTableBuilder etc.) supports now 64bit source positions. Encoding source positions in a single 64bit int together with the difference encoding in the source position table results in very little overhead for the inlining id, since only 12% of the source positions in Octane have a changed inlining id. - The class HPositionInfo was effectively dead code and is now removed. - SourcePosition has new printing and information facilities, including computing a full inlining stack. - I had to rename compiler/source-position.{h,cc} to compiler/compiler-source-position-table.{h,cc} to avoid clashes with the new src/source-position.cc file. - I wrote the new wrapper PodArray for ByteArray. It is a template working with any POD-type. This is used in DeoptimizationInputData::InliningPositions(). - I removed HInlinedFunctionInfo and HGraph::inlined_function_infos, because they were only used for the now obsolete Crankshaft inlining ids. - Crankshaft managed a list of inlined functions in Lithium: LChunk::inlined_functions. This is an analog structure to CompilationInfo::inlined_functions. So I removed LChunk::inlined_functions and made Crankshaft use CompilationInfo::inlined_functions instead, because this was necessary to register the offsets into the literal array in a uniform way. This is a safe change because LChunk::inlined_functions has no other uses and the functions in CompilationInfo::inlined_functions have a strictly longer lifespan, being created earlier (in Hydrogen already). BUG=v8:5432 Review-Url: https://codereview.chromium.org/2451853002 Cr-Commit-Position: refs/heads/master@{#40975}
2016-11-14 17:21:37 +00:00
#include "src/compiler/compiler-source-position-table.h"
#include "src/compiler/js-context-specialization.h"
#include "src/compiler/js-graph.h"
#include "src/compiler/js-heap-broker.h"
#include "src/compiler/js-operator.h"
#include "src/compiler/node-matchers.h"
#include "src/compiler/node-properties.h"
#include "src/compiler/simplified-operator.h"
#include "src/heap/factory.h"
#include "src/objects/objects-inl.h"
#include "src/objects/property.h"
#include "test/cctest/cctest.h"
#include "test/cctest/compiler/function-tester.h"
namespace v8 {
namespace internal {
namespace compiler {
class ContextSpecializationTester : public HandleAndZoneScope {
public:
explicit ContextSpecializationTester(Maybe<OuterContext> context)
: canonical_(main_isolate()),
graph_(new (main_zone()) Graph(main_zone())),
common_(main_zone()),
javascript_(main_zone()),
machine_(main_zone()),
simplified_(main_zone()),
jsgraph_(main_isolate(), graph(), common(), &javascript_, &simplified_,
&machine_),
reducer_(main_zone(), graph(), &tick_counter_),
js_heap_broker_(main_isolate(), main_zone()),
spec_(&reducer_, jsgraph(), &js_heap_broker_, context,
MaybeHandle<JSFunction>()) {}
JSContextSpecialization* spec() { return &spec_; }
Factory* factory() { return main_isolate()->factory(); }
CommonOperatorBuilder* common() { return &common_; }
JSOperatorBuilder* javascript() { return &javascript_; }
SimplifiedOperatorBuilder* simplified() { return &simplified_; }
JSGraph* jsgraph() { return &jsgraph_; }
Graph* graph() { return graph_; }
void CheckChangesToValue(Node* node, Handle<HeapObject> expected_value);
void CheckContextInputAndDepthChanges(
Node* node, Handle<Context> expected_new_context_object,
size_t expected_new_depth);
void CheckContextInputAndDepthChanges(Node* node, Node* expected_new_context,
size_t expected_new_depth);
JSHeapBroker* broker() { return &js_heap_broker_; }
private:
TickCounter tick_counter_;
CanonicalHandleScope canonical_;
Graph* graph_;
CommonOperatorBuilder common_;
JSOperatorBuilder javascript_;
MachineOperatorBuilder machine_;
SimplifiedOperatorBuilder simplified_;
JSGraph jsgraph_;
GraphReducer reducer_;
JSHeapBroker js_heap_broker_;
JSContextSpecialization spec_;
};
void ContextSpecializationTester::CheckChangesToValue(
Node* node, Handle<HeapObject> expected_value) {
Reduction r = spec()->Reduce(node);
CHECK(r.Changed());
HeapObjectMatcher match(r.replacement());
CHECK(match.HasValue());
CHECK_EQ(*match.Value(), *expected_value);
}
void ContextSpecializationTester::CheckContextInputAndDepthChanges(
Node* node, Handle<Context> expected_new_context_object,
size_t expected_new_depth) {
ContextAccess access = ContextAccessOf(node->op());
Reduction r = spec()->Reduce(node);
CHECK(r.Changed());
Node* new_context = NodeProperties::GetContextInput(r.replacement());
CHECK_EQ(IrOpcode::kHeapConstant, new_context->opcode());
HeapObjectMatcher match(new_context);
CHECK_EQ(Context::cast(*match.Value()), *expected_new_context_object);
ContextAccess new_access = ContextAccessOf(r.replacement()->op());
CHECK_EQ(new_access.depth(), expected_new_depth);
CHECK_EQ(new_access.index(), access.index());
CHECK_EQ(new_access.immutable(), access.immutable());
}
void ContextSpecializationTester::CheckContextInputAndDepthChanges(
Node* node, Node* expected_new_context, size_t expected_new_depth) {
ContextAccess access = ContextAccessOf(node->op());
Reduction r = spec()->Reduce(node);
CHECK(r.Changed());
Node* new_context = NodeProperties::GetContextInput(r.replacement());
CHECK_EQ(new_context, expected_new_context);
ContextAccess new_access = ContextAccessOf(r.replacement()->op());
CHECK_EQ(new_access.depth(), expected_new_depth);
CHECK_EQ(new_access.index(), access.index());
CHECK_EQ(new_access.immutable(), access.immutable());
}
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
static const int slot_index = Context::PREVIOUS_INDEX;
TEST(ReduceJSLoadContext0) {
ContextSpecializationTester t(Nothing<OuterContext>());
Node* start = t.graph()->NewNode(t.common()->Start(0));
t.graph()->SetStart(start);
// Make a context and initialize it a bit for this test.
Handle<Context> native = t.factory()->NewNativeContext();
Handle<Context> subcontext1 = t.factory()->NewNativeContext();
Handle<Context> subcontext2 = t.factory()->NewNativeContext();
subcontext2->set_previous(*subcontext1);
subcontext1->set_previous(*native);
Handle<Object> expected = t.factory()->InternalizeUtf8String("gboy!");
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
const int slot = Context::PREVIOUS_INDEX;
native->set(slot, *expected);
Node* const_context = t.jsgraph()->Constant(ObjectRef(t.broker(), native));
Node* deep_const_context =
t.jsgraph()->Constant(ObjectRef(t.broker(), subcontext2));
Node* param_context = t.graph()->NewNode(t.common()->Parameter(0), start);
{
// Mutable slot, constant context, depth = 0 => do nothing.
Node* load = t.graph()->NewNode(t.javascript()->LoadContext(0, 0, false),
const_context, start);
Reduction r = t.spec()->Reduce(load);
CHECK(!r.Changed());
}
{
// Mutable slot, non-constant context, depth = 0 => do nothing.
Node* load = t.graph()->NewNode(t.javascript()->LoadContext(0, 0, false),
param_context, start);
Reduction r = t.spec()->Reduce(load);
CHECK(!r.Changed());
}
{
// Mutable slot, constant context, depth > 0 => fold-in parent context.
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(2, Context::GLOBAL_EVAL_FUN_INDEX, false),
deep_const_context, start);
Reduction r = t.spec()->Reduce(load);
CHECK(r.Changed());
Node* new_context_input = NodeProperties::GetContextInput(r.replacement());
CHECK_EQ(IrOpcode::kHeapConstant, new_context_input->opcode());
HeapObjectMatcher match(new_context_input);
CHECK_EQ(*native, Context::cast(*match.Value()));
ContextAccess access = ContextAccessOf(r.replacement()->op());
CHECK_EQ(Context::GLOBAL_EVAL_FUN_INDEX, static_cast<int>(access.index()));
CHECK_EQ(0, static_cast<int>(access.depth()));
CHECK_EQ(false, access.immutable());
}
{
// Immutable slot, constant context, depth = 0 => specialize.
Node* load = t.graph()->NewNode(t.javascript()->LoadContext(0, slot, true),
const_context, start);
Reduction r = t.spec()->Reduce(load);
CHECK(r.Changed());
CHECK(r.replacement() != load);
HeapObjectMatcher match(r.replacement());
CHECK(match.HasValue());
CHECK_EQ(*expected, *match.Value());
}
// Clean up so that verifiers don't complain.
native->set(slot, Smi::zero());
}
TEST(ReduceJSLoadContext1) {
// The graph's context chain ends in the incoming context parameter:
//
// context2 <-- context1 <-- context0 (= Parameter(0))
ContextSpecializationTester t(Nothing<OuterContext>());
Node* start = t.graph()->NewNode(t.common()->Start(0));
t.graph()->SetStart(start);
Handle<ScopeInfo> empty(ScopeInfo::Empty(t.main_isolate()), t.main_isolate());
const i::compiler::Operator* create_function_context =
t.javascript()->CreateFunctionContext(empty, 42, FUNCTION_SCOPE);
Node* context0 = t.graph()->NewNode(t.common()->Parameter(0), start);
Node* context1 =
t.graph()->NewNode(create_function_context, context0, start, start);
Node* context2 =
t.graph()->NewNode(create_function_context, context1, start, start);
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(0, slot_index, false), context2, start);
CHECK(!t.spec()->Reduce(load).Changed());
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(0, slot_index, true), context2, start);
CHECK(!t.spec()->Reduce(load).Changed());
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(1, slot_index, false), context2, start);
t.CheckContextInputAndDepthChanges(load, context1, 0);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(1, slot_index, true), context2, start);
t.CheckContextInputAndDepthChanges(load, context1, 0);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(2, slot_index, false), context2, start);
t.CheckContextInputAndDepthChanges(load, context0, 0);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(2, slot_index, true), context2, start);
t.CheckContextInputAndDepthChanges(load, context0, 0);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(3, slot_index, false), context2, start);
t.CheckContextInputAndDepthChanges(load, context0, 1);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(3, slot_index, true), context2, start);
t.CheckContextInputAndDepthChanges(load, context0, 1);
}
}
TEST(ReduceJSLoadContext2) {
// The graph's context chain ends in a constant context (context_object1),
// which has another outer context (context_object0).
//
// context2 <-- context1 <-- context0 (= HeapConstant(context_object1))
// context_object1 <~~ context_object0
ContextSpecializationTester t(Nothing<OuterContext>());
Node* start = t.graph()->NewNode(t.common()->Start(0));
t.graph()->SetStart(start);
Handle<ScopeInfo> empty(ScopeInfo::Empty(t.main_isolate()), t.main_isolate());
const i::compiler::Operator* create_function_context =
t.javascript()->CreateFunctionContext(empty, 42, FUNCTION_SCOPE);
Handle<HeapObject> slot_value0 = t.factory()->InternalizeUtf8String("0");
Handle<HeapObject> slot_value1 = t.factory()->InternalizeUtf8String("1");
Handle<Context> context_object0 = t.factory()->NewNativeContext();
Handle<Context> context_object1 = t.factory()->NewNativeContext();
context_object1->set_previous(*context_object0);
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
context_object1->set(Context::EXTENSION_INDEX, *slot_value1);
Node* context0 =
t.jsgraph()->Constant(ObjectRef(t.broker(), context_object1));
Node* context1 =
t.graph()->NewNode(create_function_context, context0, start, start);
Node* context2 =
t.graph()->NewNode(create_function_context, context1, start, start);
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(0, slot_index, false), context2, start);
CHECK(!t.spec()->Reduce(load).Changed());
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(0, slot_index, true), context2, start);
CHECK(!t.spec()->Reduce(load).Changed());
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(1, slot_index, false), context2, start);
t.CheckContextInputAndDepthChanges(load, context1, 0);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(1, slot_index, true), context2, start);
t.CheckContextInputAndDepthChanges(load, context1, 0);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(2, slot_index, false), context2, start);
t.CheckContextInputAndDepthChanges(load, context0, 0);
}
{
Node* load = t.graph()->NewNode(
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
t.javascript()->LoadContext(2, Context::EXTENSION_INDEX, true),
context2, start);
t.CheckChangesToValue(load, slot_value1);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(3, slot_index, false), context2, start);
t.CheckContextInputAndDepthChanges(load, context_object0, 0);
}
{
Node* load = t.graph()->NewNode(
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
t.javascript()->LoadContext(3, Context::EXTENSION_INDEX, true),
context2, start);
t.CheckChangesToValue(load, slot_value0);
}
}
TEST(ReduceJSLoadContext3) {
// Like in ReduceJSLoadContext1, the graph's context chain ends in the
// incoming context parameter. However, this time we provide a concrete
// context for this parameter as the "specialization context". We choose
// context_object2 from ReduceJSLoadContext2 for this, so almost all test
// expectations are the same as in ReduceJSLoadContext2.
HandleAndZoneScope handle_zone_scope;
auto factory = handle_zone_scope.main_isolate()->factory();
Handle<HeapObject> slot_value0 = factory->InternalizeUtf8String("0");
Handle<HeapObject> slot_value1 = factory->InternalizeUtf8String("1");
Handle<Context> context_object0 = factory->NewNativeContext();
Handle<Context> context_object1 = factory->NewNativeContext();
context_object1->set_previous(*context_object0);
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
context_object1->set(Context::EXTENSION_INDEX, *slot_value1);
ContextSpecializationTester t(Just(OuterContext(context_object1, 0)));
Node* start = t.graph()->NewNode(t.common()->Start(2));
t.graph()->SetStart(start);
Handle<ScopeInfo> empty(ScopeInfo::Empty(t.main_isolate()),
handle_zone_scope.main_isolate());
const i::compiler::Operator* create_function_context =
t.javascript()->CreateFunctionContext(empty, 42, FUNCTION_SCOPE);
Node* context0 = t.graph()->NewNode(t.common()->Parameter(0), start);
Node* context1 =
t.graph()->NewNode(create_function_context, context0, start, start);
Node* context2 =
t.graph()->NewNode(create_function_context, context1, start, start);
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(0, slot_index, false), context2, start);
CHECK(!t.spec()->Reduce(load).Changed());
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(0, slot_index, true), context2, start);
CHECK(!t.spec()->Reduce(load).Changed());
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(1, slot_index, false), context2, start);
t.CheckContextInputAndDepthChanges(load, context1, 0);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(1, slot_index, true), context2, start);
t.CheckContextInputAndDepthChanges(load, context1, 0);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(2, slot_index, false), context2, start);
t.CheckContextInputAndDepthChanges(load, context_object1, 0);
}
{
Node* load = t.graph()->NewNode(
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
t.javascript()->LoadContext(2, Context::EXTENSION_INDEX, true),
context2, start);
t.CheckChangesToValue(load, slot_value1);
}
{
Node* load = t.graph()->NewNode(
t.javascript()->LoadContext(3, slot_index, false), context2, start);
t.CheckContextInputAndDepthChanges(load, context_object0, 0);
}
{
Node* load = t.graph()->NewNode(
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
t.javascript()->LoadContext(3, Context::EXTENSION_INDEX, true),
context2, start);
t.CheckChangesToValue(load, slot_value0);
}
}
TEST(ReduceJSStoreContext0) {
ContextSpecializationTester t(Nothing<OuterContext>());
Node* start = t.graph()->NewNode(t.common()->Start(0));
t.graph()->SetStart(start);
// Make a context and initialize it a bit for this test.
Handle<Context> native = t.factory()->NewNativeContext();
Handle<Context> subcontext1 = t.factory()->NewNativeContext();
Handle<Context> subcontext2 = t.factory()->NewNativeContext();
subcontext2->set_previous(*subcontext1);
subcontext1->set_previous(*native);
Handle<Object> expected = t.factory()->InternalizeUtf8String("gboy!");
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
const int slot = Context::PREVIOUS_INDEX;
native->set(slot, *expected);
Node* const_context = t.jsgraph()->Constant(ObjectRef(t.broker(), native));
Node* deep_const_context =
t.jsgraph()->Constant(ObjectRef(t.broker(), subcontext2));
Node* param_context = t.graph()->NewNode(t.common()->Parameter(0), start);
{
// Mutable slot, constant context, depth = 0 => do nothing.
Node* load = t.graph()->NewNode(t.javascript()->StoreContext(0, 0),
const_context, const_context, start, start);
Reduction r = t.spec()->Reduce(load);
CHECK(!r.Changed());
}
{
// Mutable slot, non-constant context, depth = 0 => do nothing.
Node* load = t.graph()->NewNode(t.javascript()->StoreContext(0, 0),
param_context, param_context, start, start);
Reduction r = t.spec()->Reduce(load);
CHECK(!r.Changed());
}
{
// Immutable slot, constant context, depth = 0 => do nothing.
Node* load = t.graph()->NewNode(t.javascript()->StoreContext(0, slot),
const_context, const_context, start, start);
Reduction r = t.spec()->Reduce(load);
CHECK(!r.Changed());
}
{
// Mutable slot, constant context, depth > 0 => fold-in parent context.
Node* load = t.graph()->NewNode(
t.javascript()->StoreContext(2, Context::GLOBAL_EVAL_FUN_INDEX),
deep_const_context, deep_const_context, start, start);
Reduction r = t.spec()->Reduce(load);
CHECK(r.Changed());
Node* new_context_input = NodeProperties::GetContextInput(r.replacement());
CHECK_EQ(IrOpcode::kHeapConstant, new_context_input->opcode());
HeapObjectMatcher match(new_context_input);
CHECK_EQ(*native, Context::cast(*match.Value()));
ContextAccess access = ContextAccessOf(r.replacement()->op());
CHECK_EQ(Context::GLOBAL_EVAL_FUN_INDEX, static_cast<int>(access.index()));
CHECK_EQ(0, static_cast<int>(access.depth()));
CHECK_EQ(false, access.immutable());
}
// Clean up so that verifiers don't complain.
native->set(slot, Smi::zero());
}
TEST(ReduceJSStoreContext1) {
ContextSpecializationTester t(Nothing<OuterContext>());
Node* start = t.graph()->NewNode(t.common()->Start(0));
t.graph()->SetStart(start);
Handle<ScopeInfo> empty(ScopeInfo::Empty(t.main_isolate()), t.main_isolate());
const i::compiler::Operator* create_function_context =
t.javascript()->CreateFunctionContext(empty, 42, FUNCTION_SCOPE);
Node* context0 = t.graph()->NewNode(t.common()->Parameter(0), start);
Node* context1 =
t.graph()->NewNode(create_function_context, context0, start, start);
Node* context2 =
t.graph()->NewNode(create_function_context, context1, start, start);
{
Node* store =
t.graph()->NewNode(t.javascript()->StoreContext(0, slot_index),
context2, context2, start, start);
CHECK(!t.spec()->Reduce(store).Changed());
}
{
Node* store =
t.graph()->NewNode(t.javascript()->StoreContext(1, slot_index),
context2, context2, start, start);
t.CheckContextInputAndDepthChanges(store, context1, 0);
}
{
Node* store =
t.graph()->NewNode(t.javascript()->StoreContext(2, slot_index),
context2, context2, start, start);
t.CheckContextInputAndDepthChanges(store, context0, 0);
}
{
Node* store =
t.graph()->NewNode(t.javascript()->StoreContext(3, slot_index),
context2, context2, start, start);
t.CheckContextInputAndDepthChanges(store, context0, 1);
}
}
TEST(ReduceJSStoreContext2) {
ContextSpecializationTester t(Nothing<OuterContext>());
Node* start = t.graph()->NewNode(t.common()->Start(0));
t.graph()->SetStart(start);
Handle<ScopeInfo> empty(ScopeInfo::Empty(t.main_isolate()), t.main_isolate());
const i::compiler::Operator* create_function_context =
t.javascript()->CreateFunctionContext(empty, 42, FUNCTION_SCOPE);
Handle<HeapObject> slot_value0 = t.factory()->InternalizeUtf8String("0");
Handle<HeapObject> slot_value1 = t.factory()->InternalizeUtf8String("1");
Handle<Context> context_object0 = t.factory()->NewNativeContext();
Handle<Context> context_object1 = t.factory()->NewNativeContext();
context_object1->set_previous(*context_object0);
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
context_object1->set(Context::EXTENSION_INDEX, *slot_value1);
Node* context0 =
t.jsgraph()->Constant(ObjectRef(t.broker(), context_object1));
Node* context1 =
t.graph()->NewNode(create_function_context, context0, start, start);
Node* context2 =
t.graph()->NewNode(create_function_context, context1, start, start);
{
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
Node* store = t.graph()->NewNode(
t.javascript()->StoreContext(0, Context::EXTENSION_INDEX), context2,
context2, start, start);
CHECK(!t.spec()->Reduce(store).Changed());
}
{
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
Node* store = t.graph()->NewNode(
t.javascript()->StoreContext(1, Context::EXTENSION_INDEX), context2,
context2, start, start);
t.CheckContextInputAndDepthChanges(store, context1, 0);
}
{
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
Node* store = t.graph()->NewNode(
t.javascript()->StoreContext(2, Context::EXTENSION_INDEX), context2,
context2, start, start);
t.CheckContextInputAndDepthChanges(store, context0, 0);
}
{
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
Node* store = t.graph()->NewNode(
t.javascript()->StoreContext(3, Context::EXTENSION_INDEX), context2,
context2, start, start);
t.CheckContextInputAndDepthChanges(store, context_object0, 0);
}
}
TEST(ReduceJSStoreContext3) {
HandleAndZoneScope handle_zone_scope;
auto factory = handle_zone_scope.main_isolate()->factory();
Handle<HeapObject> slot_value0 = factory->InternalizeUtf8String("0");
Handle<HeapObject> slot_value1 = factory->InternalizeUtf8String("1");
Handle<Context> context_object0 = factory->NewNativeContext();
Handle<Context> context_object1 = factory->NewNativeContext();
context_object1->set_previous(*context_object0);
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
context_object0->set(Context::EXTENSION_INDEX, *slot_value0);
context_object1->set(Context::EXTENSION_INDEX, *slot_value1);
ContextSpecializationTester t(Just(OuterContext(context_object1, 0)));
Node* start = t.graph()->NewNode(t.common()->Start(2));
t.graph()->SetStart(start);
Handle<ScopeInfo> empty(ScopeInfo::Empty(t.main_isolate()),
handle_zone_scope.main_isolate());
const i::compiler::Operator* create_function_context =
t.javascript()->CreateFunctionContext(empty, 42, FUNCTION_SCOPE);
Node* context0 = t.graph()->NewNode(t.common()->Parameter(0), start);
Node* context1 =
t.graph()->NewNode(create_function_context, context0, start, start);
Node* context2 =
t.graph()->NewNode(create_function_context, context1, start, start);
{
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
Node* store = t.graph()->NewNode(
t.javascript()->StoreContext(0, Context::EXTENSION_INDEX), context2,
context2, start, start);
CHECK(!t.spec()->Reduce(store).Changed());
}
{
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
Node* store = t.graph()->NewNode(
t.javascript()->StoreContext(1, Context::EXTENSION_INDEX), context2,
context2, start, start);
t.CheckContextInputAndDepthChanges(store, context1, 0);
}
{
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
Node* store = t.graph()->NewNode(
t.javascript()->StoreContext(2, Context::EXTENSION_INDEX), context2,
context2, start, start);
t.CheckContextInputAndDepthChanges(store, context_object1, 0);
}
{
Reland^2 "[runtime] Move Context::native_context to the map" This is a reland of c7c47c68f2c0eb1155cfacab785aba54f61d9218. This makes TSAN happy in addition to: Previously I presumed that the context read from a frame in the profiler was a valid context. Turns out that on non-intel we're not guaranteed that the frame is properly set up. In the case we looked at, the profiler took a sample right before writing the frame marker indicating a builtin frame, causing the "context" pointer from that frame to be a bytecode array. Since we'll read random garbage on the stack as a possible context pointer, I made the code reading the native context from it a little more defensive. Bug: v8:9860 Tbr: ulan@chromium.org, neis@chromium.org, ishell@chromium.org Original change's description: > [runtime] Move Context::native_context to the map > > Remove the native context slot from contexts by making context maps > native-context-specific. Now we require 2 loads to go from a context to the > native context, but we have 1 field fewer to store when creating contexts. > > Change-Id: I3c0d7c50c94060c4129db684f46a567de6f30e8d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1859629 > Commit-Queue: Toon Verwaest <verwaest@chromium.org> > Reviewed-by: Igor Sheludko <ishell@chromium.org> > Reviewed-by: Peter Marshall <petermarshall@chromium.org> > Reviewed-by: Maya Lekova <mslekova@chromium.org> > Reviewed-by: Georg Neis <neis@chromium.org> > Reviewed-by: Ulan Degenbaev <ulan@chromium.org> > Reviewed-by: Toon Verwaest <verwaest@chromium.org> > Cr-Commit-Position: refs/heads/master@{#64296} Change-Id: I4d0ab4cbbb23a9ae616407f17ef8f35a0b68ddb4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1864654 Commit-Queue: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Reviewed-by: Peter Marshall <petermarshall@chromium.org> Cr-Commit-Position: refs/heads/master@{#64360}
2019-10-17 15:58:38 +00:00
Node* store = t.graph()->NewNode(
t.javascript()->StoreContext(3, Context::EXTENSION_INDEX), context2,
context2, start, start);
t.CheckContextInputAndDepthChanges(store, context_object0, 0);
}
}
TEST(SpecializeJSFunction_ToConstant1) {
FunctionTester T(
"(function() { var x = 1; function inc(a)"
" { return a + x; } return inc; })()");
T.CheckCall(1.0, 0.0, 0.0);
T.CheckCall(2.0, 1.0, 0.0);
T.CheckCall(2.1, 1.1, 0.0);
}
TEST(SpecializeJSFunction_ToConstant2) {
FunctionTester T(
"(function() { var x = 1.5; var y = 2.25; var z = 3.75;"
" function f(a) { return a - x + y - z; } return f; })()");
T.CheckCall(-3.0, 0.0, 0.0);
T.CheckCall(-2.0, 1.0, 0.0);
T.CheckCall(-1.9, 1.1, 0.0);
}
TEST(SpecializeJSFunction_ToConstant3) {
FunctionTester T(
"(function() { var x = -11.5; function inc()"
" { return (function(a) { return a + x; }); }"
" return inc(); })()");
T.CheckCall(-11.5, 0.0, 0.0);
T.CheckCall(-10.5, 1.0, 0.0);
T.CheckCall(-10.4, 1.1, 0.0);
}
TEST(SpecializeJSFunction_ToConstant_uninit) {
{
FunctionTester T(
"(function() { if (false) { var x = 1; } function inc(a)"
" { return x; } return inc; })()"); // x is undefined!
i::Isolate* isolate = CcTest::i_isolate();
CHECK(
T.Call(T.Val(0.0), T.Val(0.0)).ToHandleChecked()->IsUndefined(isolate));
CHECK(
T.Call(T.Val(2.0), T.Val(0.0)).ToHandleChecked()->IsUndefined(isolate));
CHECK(T.Call(T.Val(-2.1), T.Val(0.0))
.ToHandleChecked()
->IsUndefined(isolate));
}
{
FunctionTester T(
"(function() { if (false) { var x = 1; } function inc(a)"
" { return a + x; } return inc; })()"); // x is undefined!
CHECK(T.Call(T.Val(0.0), T.Val(0.0)).ToHandleChecked()->IsNaN());
CHECK(T.Call(T.Val(2.0), T.Val(0.0)).ToHandleChecked()->IsNaN());
CHECK(T.Call(T.Val(-2.1), T.Val(0.0)).ToHandleChecked()->IsNaN());
}
}
} // namespace compiler
} // namespace internal
} // namespace v8