Commit 0336a33c authored by Sathya Gunasekaran's avatar Sathya Gunasekaran Committed by Commit Bot

Revert "[Context] Add a bit flag to indicate if extension might exist"

This reverts commit d7b67ce2.

Reason for revert: broke tsan https://logs.chromium.org/logs/v8/buildbucket/cr-buildbucket.appspot.com/8901789268326050304/+/steps/Check/0/logs/enumeration-order/0

Original change's description:
> [Context] Add a bit flag to indicate if extension might exist
> 
> Checking the bit flag instead of comparing pointers should improve performance.
> This will also allow us to remove the extension slot in Context and save memory.
> 
> Bug: v8:9744
> Change-Id: I7ab9feeadfb934955798d877d13bc0e1d78a191c
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1814918
> Commit-Queue: Victor Gomes <victorgomes@google.com>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#63906}

TBR=ulan@chromium.org,leszeks@chromium.org,victorgomes@google.com

Change-Id: I3d2261e24c9c7da5f5a1d49803361bc6f0770330
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:9744
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1816514Reviewed-by: 's avatarSathya Gunasekaran  <gsathya@chromium.org>
Commit-Queue: Sathya Gunasekaran  <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63907}
parent d7b67ce2
......@@ -2787,13 +2787,6 @@ TNode<Float64T> CodeStubAssembler::LoadDoubleWithHoleCheck(
return UncheckedCast<Float64T>(Load(machine_type, base, offset));
}
TNode<BoolT> CodeStubAssembler::LoadContextHasExtensionField(
SloppyTNode<Context> context) {
TNode<IntPtrT> value =
LoadAndUntagObjectField(context, Context::kLengthOffset);
return IsSetWord<Context::HasExtensionField>(value);
}
TNode<Object> CodeStubAssembler::LoadContextElement(
SloppyTNode<Context> context, int slot_index) {
int offset = Context::SlotOffset(slot_index);
......
......@@ -1436,7 +1436,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
ElementsKind elements_kind);
// Context manipulation
TNode<BoolT> LoadContextHasExtensionField(SloppyTNode<Context> context);
TNode<Object> LoadContextElement(SloppyTNode<Context> context,
int slot_index);
TNode<Object> LoadContextElement(SloppyTNode<Context> context,
......
......@@ -1421,7 +1421,7 @@ Handle<Context> Factory::NewContext(RootIndex map_root_index, int size,
Map map = Map::cast(isolate()->root(map_root_index));
HeapObject result = AllocateRawWithImmortalMap(size, allocation, map);
Handle<Context> context(Context::cast(result), isolate());
context->initialize_length_and_extension_bit(variadic_part_length);
context->set_length(variadic_part_length);
DCHECK_EQ(context->SizeFromMap(map), size);
if (size > Context::kTodoHeaderSize) {
ObjectSlot start = context->RawField(Context::kTodoHeaderSize);
......
......@@ -205,32 +205,27 @@ void InterpreterAssembler::GotoIfHasContextExtensionUpToDepth(
TVARIABLE(Uint32T, cur_depth, depth);
Label context_search(this, {&cur_depth, &cur_context});
Label no_extension(this);
// Loop until the depth is 0.
Goto(&context_search);
BIND(&context_search);
{
// Check if context has an extension slot
TNode<BoolT> has_extension =
LoadContextHasExtensionField(cur_context.value());
GotoIfNot(has_extension, &no_extension);
// TODO(leszeks): We only need to do this check if the context had a sloppy
// eval, we could pass in a context chain bitmask to figure out which
// contexts actually need to be checked.
// Jump to the target if the extension slot is not a hole.
TNode<Object> extension_slot =
LoadContextElement(cur_context.value(), Context::EXTENSION_INDEX);
Branch(TaggedNotEqual(extension_slot, TheHoleConstant()), target,
&no_extension);
BIND(&no_extension);
{
cur_depth = Unsigned(Int32Sub(cur_depth.value(), Int32Constant(1)));
cur_context = CAST(
LoadContextElement(cur_context.value(), Context::PREVIOUS_INDEX));
// Jump to the target if the extension slot is not a hole.
GotoIf(TaggedNotEqual(extension_slot, TheHoleConstant()), target);
GotoIf(Word32NotEqual(cur_depth.value(), Int32Constant(0)),
&context_search);
}
cur_depth = Unsigned(Int32Sub(cur_depth.value(), Int32Constant(1)));
cur_context =
CAST(LoadContextElement(cur_context.value(), Context::PREVIOUS_INDEX));
GotoIf(Word32NotEqual(cur_depth.value(), Int32Constant(0)),
&context_search);
}
}
......
......@@ -47,23 +47,10 @@ Context ScriptContextTable::get_context(int i) const {
OBJECT_CONSTRUCTORS_IMPL(Context, HeapObject)
NEVER_READ_ONLY_SPACE_IMPL(Context)
CAST_ACCESSOR(Context)
SMI_ACCESSORS(Context, length_and_extension_flag, kLengthOffset)
SMI_ACCESSORS(Context, length, kLengthOffset)
CAST_ACCESSOR(NativeContext)
int Context::length() const {
return LengthField::decode(length_and_extension_flag());
}
void Context::initialize_length_and_extension_bit(int len,
Context::HasExtension flag) {
DCHECK(LengthField::is_valid(len));
int value = 0;
value = LengthField::update(value, len);
value = HasExtensionField::update(value, flag == Context::HasExtension::kYes);
set_length_and_extension_flag(value);
}
Object Context::get(int index) const {
Isolate* isolate = GetIsolateForPtrCompr(*this);
return get(isolate, index);
......@@ -107,20 +94,11 @@ void Context::set_previous(Context context) { set(PREVIOUS_INDEX, context); }
Object Context::next_context_link() { return get(Context::NEXT_CONTEXT_LINK); }
bool Context::has_extension() {
return static_cast<bool>(
HasExtensionField::decode(length_and_extension_flag())) &&
!extension().IsTheHole();
}
bool Context::has_extension() { return !extension().IsTheHole(); }
HeapObject Context::extension() {
return HeapObject::cast(get(EXTENSION_INDEX));
}
void Context::set_extension(HeapObject object) {
set(EXTENSION_INDEX, object);
set_length_and_extension_flag(
HasExtensionField::update(length_and_extension_flag(), true));
}
void Context::set_extension(HeapObject object) { set(EXTENSION_INDEX, object); }
NativeContext Context::native_context() const {
Object result = get(NATIVE_CONTEXT_INDEX);
......
......@@ -453,15 +453,9 @@ class Context : public HeapObject {
DECL_CAST(Context)
enum class HasExtension { kYes, kNo };
// [length]: length of the context.
V8_INLINE int length() const;
V8_INLINE void initialize_length_and_extension_bit(
int len, HasExtension flag = HasExtension::kNo);
using LengthField = BitField<int, 0, kSmiValueSize - 2>;
using HasExtensionField = BitField<int, kSmiValueSize - 2, 1>;
V8_INLINE void set_length(int value);
// Setter and getter for elements.
V8_INLINE Object get(int index) const;
......@@ -668,7 +662,6 @@ class Context : public HeapObject {
#endif
OBJECT_CONSTRUCTORS(Context, HeapObject);
DECL_INT_ACCESSORS(length_and_extension_flag)
};
class NativeContext : public Context {
......
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