Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LOAD_METHOD_TYPE #499

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion runtime/bytecode.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ namespace py {
V(UNUSED_BYTECODE_170, 170, doInvalidBytecode) \
V(UNUSED_BYTECODE_171, 171, doInvalidBytecode) \
V(UNUSED_BYTECODE_172, 172, doInvalidBytecode) \
V(UNUSED_BYTECODE_173, 173, doInvalidBytecode) \
V(LOAD_METHOD_TYPE, 173, doLoadMethodType) \
V(LOAD_METHOD_MODULE, 174, doLoadMethodModule) \
V(CALL_FUNCTION_TYPE_INIT, 175, doCallFunctionTypeInit) \
V(CALL_FUNCTION_TYPE_NEW, 176, doCallFunctionTypeNew) \
Expand Down Expand Up @@ -399,6 +399,7 @@ inline bool isByteCodeWithCache(const Bytecode bc) {
case LOAD_METHOD_ANAMORPHIC:
case LOAD_METHOD_INSTANCE_FUNCTION:
case LOAD_METHOD_POLYMORPHIC:
case LOAD_METHOD_TYPE:
case STORE_ATTR_INSTANCE:
case STORE_ATTR_INSTANCE_OVERFLOW:
case STORE_ATTR_INSTANCE_OVERFLOW_UPDATE:
Expand Down
19 changes: 19 additions & 0 deletions runtime/ic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,25 @@ void icUpdateMethodModule(Thread* thread, const MutableTuple& caches,
icInsertDependentToValueCellDependencyLink(thread, dependent, value_cell);
}

void icUpdateMethodType(Thread* thread, const MutableTuple& caches, word cache,
const Object& receiver, const ValueCell& value_cell,
const Function& dependent) {
DCHECK(icIsCacheEmpty(caches, cache), "cache must be empty\n");
HandleScope scope(thread);
word index = cache * kIcPointersPerEntry;
Type type(&scope, *receiver);
caches.atPut(index + kIcEntryKeyOffset,
SmallInt::fromWord(static_cast<word>(type.instanceLayoutId())));
caches.atPut(index + kIcEntryValueOffset, *value_cell);
RawMutableBytes bytecode =
RawMutableBytes::cast(dependent.rewrittenBytecode());
word pc = thread->currentFrame()->virtualPC() - kCodeUnitSize;
DCHECK(bytecode.byteAt(pc) == LOAD_METHOD_ANAMORPHIC,
"current opcode must be LOAD_METHOD_ANAMORPHIC");
bytecode.byteAtPut(pc, LOAD_METHOD_TYPE);
icInsertDependentToValueCellDependencyLink(thread, dependent, value_cell);
}

void icUpdateAttrType(Thread* thread, const MutableTuple& caches, word cache,
const Object& receiver, const Object& selector,
const Object& value, const Function& dependent) {
Expand Down
5 changes: 5 additions & 0 deletions runtime/ic.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ void icUpdateMethodModule(Thread* thread, const MutableTuple& caches,
const ValueCell& value_cell,
const Function& dependent);

void icUpdateMethodType(Thread* thread, const MutableTuple& caches, word cache,
const Object& receiver, const ValueCell& value_cell,
const Function& dependent);

void icUpdateAttrType(Thread* thread, const MutableTuple& caches, word cache,
const Object& receiver, const Object& selector,
const Object& value, const Function& dependent);
Expand Down Expand Up @@ -351,6 +355,7 @@ class IcIterator {
case LOAD_ATTR_ANAMORPHIC:
case LOAD_METHOD_ANAMORPHIC:
case LOAD_METHOD_INSTANCE_FUNCTION:
case LOAD_METHOD_TYPE:
case LOAD_METHOD_POLYMORPHIC:
case LOAD_TYPE:
case STORE_ATTR_INSTANCE:
Expand Down
29 changes: 29 additions & 0 deletions runtime/interpreter-gen-x64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1515,6 +1515,35 @@ void emitHandler<LOAD_METHOD_MODULE>(EmitEnv* env) {
emitJumpToGenericHandler(env);
}

template <>
void emitHandler<LOAD_METHOD_TYPE>(EmitEnv* env) {
ScratchReg r_base(env);
ScratchReg r_layout_id(env);
ScratchReg r_function(env);
ScratchReg r_caches(env);
Label slow_path;

__ popq(r_base);
emitJumpIfNotHeapObjectWithLayoutId(env, r_base, LayoutId::kType, &slow_path);
__ movq(r_layout_id,
Address(r_base, heapObjectDisp(Type::kInstanceLayoutIdOffset)));
__ movq(r_caches, Address(env->frame, Frame::kCachesOffset));
emitIcLookupMonomorphic(env, &slow_path, r_function, r_layout_id, r_caches);
__ movq(r_function,
Address(r_function, heapObjectDisp(ValueCell::kValueOffset)));
emitPushImmediate(env, Unbound::object().raw());
__ pushq(r_function);
emitNextOpcode(env);

__ bind(&slow_path);
__ pushq(r_base);
if (env->in_jit) {
emitJumpToDeopt(env);
return;
}
emitJumpToGenericHandler(env);
}

template <>
void emitHandler<LOAD_METHOD_POLYMORPHIC>(EmitEnv* env) {
ScratchReg r_base(env);
Expand Down
127 changes: 127 additions & 0 deletions runtime/interpreter-test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6969,6 +6969,133 @@ def invalidate():
EXPECT_EQ(rewrittenBytecodeOpAt(bytecode, 4), LOAD_METHOD_MODULE);
}

TEST_F(InterpreterTest, LoadMethodTypeCachedModuleFunction) {
EXPECT_FALSE(runFromCStr(runtime_, R"(
class C:
def foo(self):
return 123

class D:
def foo(self):
return 456

class E:
def foo(self, other):
return 789

def test(cls, obj):
return cls.foo(obj)

c = C()
d = D()
e = E()
c_cached = C.foo
d_cached = D.foo
)")
.isError());
HandleScope scope(thread_);
Function test_function(&scope, mainModuleAt(runtime_, "test"));
Function expected_c(&scope, mainModuleAt(runtime_, "c_cached"));
Function expected_d(&scope, mainModuleAt(runtime_, "d_cached"));
Type type_c(&scope, mainModuleAt(runtime_, "C"));
Object c(&scope, mainModuleAt(runtime_, "c"));
Type type_d(&scope, mainModuleAt(runtime_, "D"));
Object d(&scope, mainModuleAt(runtime_, "d"));
Object e(&scope, mainModuleAt(runtime_, "e"));
MutableBytes bytecode(&scope, test_function.rewrittenBytecode());
ASSERT_EQ(rewrittenBytecodeOpAt(bytecode, 1), LOAD_METHOD_ANAMORPHIC);

// Cache miss.
MutableTuple caches(&scope, test_function.caches());
word cache_index =
rewrittenBytecodeCacheAt(bytecode, 1) * kIcPointersPerEntry;
Object key(&scope, caches.at(cache_index + kIcEntryKeyOffset));
EXPECT_EQ(*key, NoneType::object());

// Call.
EXPECT_TRUE(isIntEqualsWord(
Interpreter::call2(thread_, test_function, type_c, c), 123));
EXPECT_EQ(rewrittenBytecodeOpAt(bytecode, 1), LOAD_METHOD_TYPE);

// Cached.
key = caches.at(cache_index + kIcEntryKeyOffset);
EXPECT_TRUE(
isIntEqualsWord(*key, static_cast<word>(type_c.instanceLayoutId())));
Object value(&scope, caches.at(cache_index + kIcEntryValueOffset));
ASSERT_TRUE(value.isValueCell());
EXPECT_EQ(ValueCell::cast(*value).value(), *expected_c);

// Call.
EXPECT_TRUE(isIntEqualsWord(
Interpreter::call2(thread_, test_function, type_d, d), 456));
EXPECT_EQ(rewrittenBytecodeOpAt(bytecode, 1), LOAD_METHOD_TYPE);

// Cache miss and re-cache.
key = caches.at(cache_index + kIcEntryKeyOffset);
EXPECT_TRUE(
isIntEqualsWord(*key, static_cast<word>(type_d.instanceLayoutId())));
value = caches.at(cache_index + kIcEntryValueOffset);
ASSERT_TRUE(value.isValueCell());
EXPECT_EQ(ValueCell::cast(*value).value(), *expected_d);

// Call and rewrite.
Object none(&scope, NoneType::object());
EXPECT_TRUE(isIntEqualsWord(
Interpreter::call2(thread_, test_function, e, none), 789));
EXPECT_EQ(rewrittenBytecodeOpAt(bytecode, 1), LOAD_METHOD_INSTANCE_FUNCTION);
}

TEST_F(InterpreterTest, LoadMethodTypeGetsEvicted) {
EXPECT_FALSE(runFromCStr(runtime_, R"(
import sys

class C:
def foo():
return 123

def test(cls):
return cls.foo()

def invalidate():
del C.foo
)")
.isError());
HandleScope scope(thread_);
Function test_function(&scope, mainModuleAt(runtime_, "test"));
Function invalidate_function(&scope, mainModuleAt(runtime_, "invalidate"));
Type type_c(&scope, mainModuleAt(runtime_, "C"));
MutableBytes bytecode(&scope, test_function.rewrittenBytecode());
ASSERT_EQ(rewrittenBytecodeOpAt(bytecode, 1), LOAD_METHOD_ANAMORPHIC);

// Cache miss.
MutableTuple caches(&scope, test_function.caches());
word cache_index =
rewrittenBytecodeCacheAt(bytecode, 1) * kIcPointersPerEntry;
Object key(&scope, caches.at(cache_index + kIcEntryKeyOffset));
EXPECT_EQ(*key, NoneType::object());

// Call.
EXPECT_TRUE(
isIntEqualsWord(Interpreter::call1(thread_, test_function, type_c), 123));
EXPECT_EQ(rewrittenBytecodeOpAt(bytecode, 1), LOAD_METHOD_TYPE);

// Update type.
ASSERT_TRUE(Interpreter::call0(thread_, invalidate_function).isNoneType());

// Cache is empty.
key = caches.at(cache_index + kIcEntryKeyOffset);
EXPECT_TRUE(key.isNoneType());

// Cache miss.
EXPECT_EQ(rewrittenBytecodeOpAt(bytecode, 1), LOAD_METHOD_TYPE);
EXPECT_TRUE(raisedWithStr(Interpreter::call1(thread_, test_function, type_c),
LayoutId::kAttributeError,
"type object 'C' has no attribute 'foo'"));

// Bytecode gets rewritten after next call.
EXPECT_EQ(rewrittenBytecodeOpAt(bytecode, 1), LOAD_METHOD_ANAMORPHIC);
}

TEST_F(InterpreterTest, LoadMethodCachedDoesNotCacheProperty) {
HandleScope scope(thread_);
EXPECT_FALSE(runFromCStr(runtime_, R"(
Expand Down
39 changes: 38 additions & 1 deletion runtime/interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4011,6 +4011,8 @@ HANDLER_INLINE Continue Interpreter::doLoadAttrType(Thread* thread, word arg) {
if (SmallInt::fromWord(id) == layout_id) {
RawObject result = caches.at(index + kIcEntryValueOffset);
DCHECK(result.isValueCell(), "cached value is not a value cell");
DCHECK(!ValueCell::cast(result).isPlaceholder(),
"cached value cell is empty");
thread->stackSetTop(ValueCell::cast(result).value());
return Continue::NEXT;
}
Expand Down Expand Up @@ -5276,7 +5278,7 @@ Continue Interpreter::loadMethodUpdateCache(Thread* thread, word arg,
thread, receiver, name, &kind, &location));
if (result.isErrorException()) return Continue::UNWIND;
if (kind != LoadAttrKind::kInstanceFunction &&
kind != LoadAttrKind::kModule) {
kind != LoadAttrKind::kModule && kind != LoadAttrKind::kType) {
thread->stackPush(*result);
thread->stackSetAt(1, Unbound::object());
return Continue::NEXT;
Expand All @@ -5302,12 +5304,22 @@ Continue Interpreter::loadMethodUpdateCache(Thread* thread, word arg,
thread->stackSetAt(1, Unbound::object());
return Continue::NEXT;
}
case LoadAttrKind::kType: {
DCHECK(location.isValueCell(), "location must be ValueCell");
ValueCell value_cell(&scope, *location);
icUpdateMethodType(thread, caches, cache, receiver, value_cell,
dependent);
thread->stackPush(*result);
thread->stackSetAt(1, Unbound::object());
return Continue::NEXT;
}
default:
break;
}
} else {
DCHECK(currentBytecode(thread) == LOAD_METHOD_INSTANCE_FUNCTION ||
currentBytecode(thread) == LOAD_METHOD_MODULE ||
currentBytecode(thread) == LOAD_METHOD_TYPE ||
currentBytecode(thread) == LOAD_METHOD_POLYMORPHIC,
"unexpected opcode %s", kBytecodeNames[currentBytecode(thread)]);
switch (kind) {
Expand Down Expand Up @@ -5378,6 +5390,31 @@ HANDLER_INLINE Continue Interpreter::doLoadMethodModule(Thread* thread,
return retryLoadMethodCached(thread, arg, cache);
}

HANDLER_INLINE Continue Interpreter::doLoadMethodType(Thread* thread,
word arg) {
Frame* frame = thread->currentFrame();
RawObject receiver = thread->stackTop();
RawMutableTuple caches = MutableTuple::cast(frame->caches());
word cache = currentCacheIndex(frame);
word index = cache * kIcPointersPerEntry;
RawObject layout_id = caches.at(index + kIcEntryKeyOffset);
Runtime* runtime = thread->runtime();
if (runtime->isInstanceOfType(receiver)) {
word id = static_cast<word>(receiver.rawCast<RawType>().instanceLayoutId());
if (SmallInt::fromWord(id) == layout_id) {
RawObject result = caches.at(index + kIcEntryValueOffset);
DCHECK(result.isValueCell(), "cached value is not a value cell");
DCHECK(!ValueCell::cast(result).isPlaceholder(),
"cached value cell is empty");
thread->stackPush(ValueCell::cast(result).value());
thread->stackSetAt(1, Unbound::object());
return Continue::NEXT;
}
}
EVENT_CACHE(LOAD_METHOD_TYPE);
return retryLoadMethodCached(thread, arg, cache);
}

HANDLER_INLINE Continue
Interpreter::doLoadMethodInstanceFunction(Thread* thread, word arg) {
Frame* frame = thread->currentFrame();
Expand Down
1 change: 1 addition & 0 deletions runtime/interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ class Interpreter {
static Continue doLoadMethodAnamorphic(Thread* thread, word arg);
static Continue doLoadMethodInstanceFunction(Thread* thread, word arg);
static Continue doLoadMethodModule(Thread* thread, word arg);
static Continue doLoadMethodType(Thread* thread, word arg);
static Continue doLoadMethodPolymorphic(Thread* thread, word arg);
static Continue doLoadName(Thread* thread, word arg);
static Continue doLoadType(Thread* thread, word arg);
Expand Down