Skip to content

[mypyc] Borrow references during chained attribute access #12805

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

Merged
merged 4 commits into from
May 18, 2022
Merged
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
2 changes: 1 addition & 1 deletion mypyc/codegen/emitfunc.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ def visit_get_attr(self, op: GetAttr) -> None:
'PyErr_SetString({}, "attribute {} of {} undefined");'.format(
exc_class, repr(op.attr), repr(cl.name)))

if attr_rtype.is_refcounted:
if attr_rtype.is_refcounted and not op.is_borrowed:
if not merged_branch and not always_defined:
self.emitter.emit_line('} else {')
self.emitter.emit_inc_ref(dest, attr_rtype)
Expand Down
3 changes: 2 additions & 1 deletion mypyc/ir/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -599,13 +599,14 @@ class GetAttr(RegisterOp):

error_kind = ERR_MAGIC

def __init__(self, obj: Value, attr: str, line: int) -> None:
def __init__(self, obj: Value, attr: str, line: int, *, borrow: bool = False) -> None:
super().__init__(line)
self.obj = obj
self.attr = attr
assert isinstance(obj.type, RInstance), 'Attribute access not supported: %s' % obj.type
self.class_type = obj.type
self.type = obj.type.attr_type(attr)
self.is_borrowed = borrow

def sources(self) -> List[Value]:
return [self.obj]
Expand Down
6 changes: 5 additions & 1 deletion mypyc/ir/pprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ def visit_load_literal(self, op: LoadLiteral) -> str:
return self.format('%r = %s%s', op, prefix, repr(op.value))

def visit_get_attr(self, op: GetAttr) -> str:
return self.format('%r = %r.%s', op, op.obj, op.attr)
if op.is_borrowed:
borrow = 'borrow '
else:
borrow = ''
return self.format('%r = %s%r.%s', op, borrow, op.obj, op.attr)

def visit_set_attr(self, op: SetAttr) -> str:
if op.is_init:
Expand Down
19 changes: 16 additions & 3 deletions mypyc/irbuild/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ def __init__(self,
# can also do quick lookups.
self.imports: OrderedDict[str, None] = OrderedDict()

self.can_borrow = False

# High-level control

def set_module(self, module_name: str, module_path: str) -> None:
Expand All @@ -152,15 +154,23 @@ def set_module(self, module_name: str, module_path: str) -> None:
self.module_path = module_path

@overload
def accept(self, node: Expression) -> Value: ...
def accept(self, node: Expression, *, can_borrow: bool = False) -> Value: ...

@overload
def accept(self, node: Statement) -> None: ...

def accept(self, node: Union[Statement, Expression]) -> Optional[Value]:
"""Transform an expression or a statement."""
def accept(self, node: Union[Statement, Expression], *,
can_borrow: bool = False) -> Optional[Value]:
"""Transform an expression or a statement.

If can_borrow is true, prefer to generate a borrowed reference.
Borrowed references are faster since they don't require reference count
manipulation, but they are only safe to use in specific contexts.
"""
with self.catch_errors(node.line):
if isinstance(node, Expression):
old_can_borrow = self.can_borrow
self.can_borrow = can_borrow
try:
res = node.accept(self.visitor)
res = self.coerce(res, self.node_type(node), node.line)
Expand All @@ -170,6 +180,9 @@ def accept(self, node: Union[Statement, Expression]) -> Optional[Value]:
# from causing more downstream trouble.
except UnsupportedException:
res = Register(self.node_type(node))
self.can_borrow = old_can_borrow
if not can_borrow:
self.builder.flush_keep_alives()
return res
else:
try:
Expand Down
18 changes: 15 additions & 3 deletions mypyc/irbuild/expression.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
Value, Register, TupleGet, TupleSet, BasicBlock, Assign, LoadAddress, RaiseStandardError
)
from mypyc.ir.rtypes import (
RTuple, object_rprimitive, is_none_rprimitive, int_rprimitive, is_int_rprimitive
RTuple, RInstance, object_rprimitive, is_none_rprimitive, int_rprimitive, is_int_rprimitive
)
from mypyc.ir.func_ir import FUNC_CLASSMETHOD, FUNC_STATICMETHOD
from mypyc.irbuild.format_str_tokenizer import (
Expand Down Expand Up @@ -130,8 +130,19 @@ def transform_member_expr(builder: IRBuilder, expr: MemberExpr) -> Value:
if isinstance(expr.node, MypyFile) and expr.node.fullname in builder.imports:
return builder.load_module(expr.node.fullname)

obj = builder.accept(expr.expr)
obj_rtype = builder.node_type(expr.expr)
if (isinstance(obj_rtype, RInstance)
and obj_rtype.class_ir.is_ext_class
and obj_rtype.class_ir.has_attr(expr.name)
and not obj_rtype.class_ir.get_method(expr.name)):
# Direct attribute access -> can borrow object
can_borrow = True
else:
can_borrow = False
obj = builder.accept(expr.expr, can_borrow=can_borrow)

rtype = builder.node_type(expr)

# Special case: for named tuples transform attribute access to faster index access.
typ = get_proper_type(builder.types.get(expr.expr))
if isinstance(typ, TupleType) and typ.partial_fallback.type.is_named_tuple:
Expand All @@ -142,7 +153,8 @@ def transform_member_expr(builder: IRBuilder, expr: MemberExpr) -> Value:

check_instance_attribute_access_through_class(builder, expr, typ)

return builder.builder.get_attr(obj, expr.name, rtype, expr.line)
borrow = can_borrow and builder.can_borrow
return builder.builder.get_attr(obj, expr.name, rtype, expr.line, borrow=borrow)


def check_instance_attribute_access_through_class(builder: IRBuilder,
Expand Down
15 changes: 13 additions & 2 deletions mypyc/irbuild/ll_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ def __init__(
self.blocks: List[BasicBlock] = []
# Stack of except handler entry blocks
self.error_handlers: List[Optional[BasicBlock]] = [None]
# Values that we need to keep alive as long as we have borrowed
# temporaries. Use flush_keep_alives() to mark the end of the live range.
self.keep_alives: List[Value] = []

# Basic operations

Expand Down Expand Up @@ -145,6 +148,11 @@ def self(self) -> Register:
"""
return self.args[0]

def flush_keep_alives(self) -> None:
if self.keep_alives:
self.add(KeepAlive(self.keep_alives[:]))
self.keep_alives = []

# Type conversions

def box(self, src: Value) -> Value:
Expand Down Expand Up @@ -219,11 +227,14 @@ def coerce_nullable(self, src: Value, target_type: RType, line: int) -> Value:

# Attribute access

def get_attr(self, obj: Value, attr: str, result_type: RType, line: int) -> Value:
def get_attr(self, obj: Value, attr: str, result_type: RType, line: int, *,
borrow: bool = False) -> Value:
"""Get a native or Python attribute of an object."""
if (isinstance(obj.type, RInstance) and obj.type.class_ir.is_ext_class
and obj.type.class_ir.has_attr(attr)):
return self.add(GetAttr(obj, attr, line))
if borrow:
self.keep_alives.append(obj)
return self.add(GetAttr(obj, attr, line, borrow=borrow))
elif isinstance(obj.type, RUnion):
return self.union_get_attr(obj, obj.type, attr, result_type, line)
else:
Expand Down
48 changes: 48 additions & 0 deletions mypyc/test-data/irbuild-classes.test
Original file line number Diff line number Diff line change
Expand Up @@ -1240,3 +1240,51 @@ L0:
r0 = ''
__mypyc_self__.s = r0
return 1

[case testBorrowAttribute]
def f(d: D) -> int:
return d.c.x

class C:
x: int
class D:
c: C
[out]
def f(d):
d :: __main__.D
r0 :: __main__.C
r1 :: int
L0:
r0 = borrow d.c
r1 = r0.x
keep_alive d
return r1

[case testNoBorrowOverPropertyAccess]
class C:
d: D
class D:
@property
def e(self) -> E:
return E()
class E:
x: int
def f(c: C) -> int:
return c.d.e.x
[out]
def D.e(self):
self :: __main__.D
r0 :: __main__.E
L0:
r0 = E()
return r0
def f(c):
c :: __main__.C
r0 :: __main__.D
r1 :: __main__.E
r2 :: int
L0:
r0 = c.d
r1 = r0.e
r2 = r1.x
return r2
33 changes: 33 additions & 0 deletions mypyc/test-data/refcount.test
Original file line number Diff line number Diff line change
Expand Up @@ -917,3 +917,36 @@ L0:
r5 = unbox(int, r4)
dec_ref r4
return r5

[case testBorrowAttribute]
def g() -> int:
d = D()
return d.c.x

def f(d: D) -> int:
return d.c.x

class C:
x: int
class D:
c: C
[out]
def g():
r0, d :: __main__.D
r1 :: __main__.C
r2 :: int
L0:
r0 = D()
d = r0
r1 = borrow d.c
r2 = r1.x
dec_ref d
return r2
def f(d):
d :: __main__.D
r0 :: __main__.C
r1 :: int
L0:
r0 = borrow d.c
r1 = r0.x
return r1