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.

Sign up for GitHub

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

[mypyc] Avoid boxing/unboxing when coercing between tuple types #14899

Merged
merged 2 commits into from
Mar 20, 2023
Merged

[mypyc] Avoid boxing/unboxing when coercing between tuple types #14899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1fd6093
[mypyc] Avoid boxing/unboxing when coercing between tuple types
JukkaL Mar 12, 2023
72d9a9c
Only run test on 64 bit archictecture
JukkaL Mar 14, 2023
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
33 changes: 28 additions & 5 deletions mypyc/irbuild/ll_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
SetMem,
Truncate,
TupleGet,
TupleSet,
Unbox,
Unreachable,
Value,
Expand Down Expand Up @@ -354,11 +355,33 @@ def coerce(
return Float(float(src.value))
elif is_tagged(src_type) and is_float_rprimitive(target_type):
return self.int_to_float(src, line)
else:
# To go from one unboxed type to another, we go through a boxed
# in-between value, for simplicity.
tmp = self.box(src)
return self.unbox_or_cast(tmp, target_type, line)
elif (
isinstance(src_type, RTuple)
and isinstance(target_type, RTuple)
and len(src_type.types) == len(target_type.types)
):
# Coerce between two tuple types by coercing each item separately
values = []
for i in range(len(src_type.types)):
v = None
if isinstance(src, TupleSet):
item = src.items[i]
# We can't reuse register values, since they can be modified.
if not isinstance(item, Register):
Comment on lines +369 to +370
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a blocking comment, moreso for my own learning. What situations exist where a register is modified during the coerce operation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is about code like this:

    t = (n, 0)
    n += 1
    t2: tuple[float, float] = t

We can't use n to refer to the first item of t when constructing t2, since n was incremented on line 2.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right. I should've thought of that, painfully obvious now you show it. Thanks!

v = item
if v is None:
v = TupleGet(src, i)
self.add(v)
values.append(v)
return self.add(
TupleSet(
[self.coerce(v, t, line) for v, t in zip(values, target_type.types)], line
)
)
# To go between any other unboxed types, we go through a boxed
# in-between value, for simplicity.
tmp = self.box(src)
return self.unbox_or_cast(tmp, target_type, line)
if (not src_type.is_unboxed and target_type.is_unboxed) or not is_subtype(
src_type, target_type
):
Expand Down
20 changes: 20 additions & 0 deletions mypyc/test-data/irbuild-float.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,3 +475,23 @@ def init(n: int) -> None:
# narrowing assignments, generate errors here
x: float = n # E: Incompatible value representations in assignment (expression has type "int", variable has type "float")
y: float = 5 # E: Incompatible value representations in assignment (expression has type "int", variable has type "float")

[case testFloatCoerceTupleFromIntValues]
from __future__ import annotations

def f(x: int) -> None:
t: tuple[float, float, float] = (x, 2.5, -7)
[out]
def f(x):
x :: int
r0 :: tuple[int, float, int]
r1 :: int
r2 :: float
r3, t :: tuple[float, float, float]
L0:
r0 = (x, 2.5, -14)
r1 = r0[0]
r2 = CPyFloat_FromTagged(r1)
r3 = (r2, 2.5, -7.0)
t = r3
return 1
84 changes: 78 additions & 6 deletions mypyc/test-data/irbuild-i64.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,6 @@ def f(x: i64, y: i64) -> Tuple[i64, i64]:
return x, y

def g() -> Tuple[i64, i64]:
# TODO: Avoid boxing and unboxing
return 1, 2

def h() -> i64:
Expand All @@ -666,13 +665,11 @@ L0:
return r0
def g():
r0 :: tuple[int, int]
r1 :: object
r2 :: tuple[int64, int64]
r1 :: tuple[int64, int64]
L0:
r0 = (2, 4)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little unfortunate that this remains in the IR, but it's nothing DCE can't fix if we ever implement it.

tuple_T288 CPyDef_g(void) {
    tuple_T2II cpy_r_r0;
    tuple_T288 cpy_r_r1;
CPyL0: ;
    cpy_r_r0.f0 = 2;
    cpy_r_r0.f1 = 4;
    CPyTagged_INCREF(cpy_r_r0.f0);
    CPyTagged_INCREF(cpy_r_r0.f1);
    CPyTagged_DECREF(cpy_r_r0.f0);
    CPyTagged_DECREF(cpy_r_r0.f1);
    cpy_r_r1.f0 = 1;
    cpy_r_r1.f1 = 2;
    return cpy_r_r1;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The C compiler may optimize some of these away. Also, optimizing these shouldn't be too difficult in mypyc. Added mypyc/mypyc#983 to track this.

r1 = box(tuple[int, int], r0)
r2 = unbox(tuple[int64, int64], r1)
return r2
r1 = (1, 2)
return r1
def h():
r0 :: tuple[int64, int64]
r1, x, r2, y :: int64
Expand Down Expand Up @@ -2081,3 +2078,78 @@ L2:
r6 = r5.a
keep_alive x
return r6

[case testI64ConvertBetweenTuples_64bit]
from __future__ import annotations
from mypy_extensions import i64

def f(t: tuple[int, i64, int]) -> None:
tt: tuple[int, i64, i64] = t

def g(n: int) -> None:
t: tuple[i64, i64] = (1, n)
[out]
def f(t):
t :: tuple[int, int64, int]
r0 :: int
r1 :: int64
r2 :: int
r3 :: native_int
r4 :: bit
r5, r6 :: int64
r7 :: ptr
r8 :: c_ptr
r9 :: int64
r10, tt :: tuple[int, int64, int64]
L0:
r0 = t[0]
r1 = t[1]
r2 = t[2]
r3 = r2 & 1
r4 = r3 == 0
if r4 goto L1 else goto L2 :: bool
L1:
r5 = r2 >> 1
r6 = r5
goto L3
L2:
r7 = r2 ^ 1
r8 = r7
r9 = CPyLong_AsInt64(r8)
r6 = r9
keep_alive r2
L3:
r10 = (r0, r1, r6)
tt = r10
return 1
def g(n):
n :: int
r0 :: tuple[int, int]
r1 :: int
r2 :: native_int
r3 :: bit
r4, r5 :: int64
r6 :: ptr
r7 :: c_ptr
r8 :: int64
r9, t :: tuple[int64, int64]
L0:
r0 = (2, n)
r1 = r0[1]
r2 = r1 & 1
r3 = r2 == 0
if r3 goto L1 else goto L2 :: bool
L1:
r4 = r1 >> 1
r5 = r4
goto L3
L2:
r6 = r1 ^ 1
r7 = r6
r8 = CPyLong_AsInt64(r7)
r5 = r8
keep_alive r1
L3:
r9 = (1, r5)
t = r9
return 1
8 changes: 8 additions & 0 deletions mypyc/test-data/run-floats.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Test cases for floats (compile and run)

[case testFloatOps]
from __future__ import annotations
from typing import Any, cast
from typing_extensions import Final
from testutil import assertRaises, float_vals, FLOAT_MAGIC
Expand Down Expand Up @@ -329,6 +330,13 @@ def test_undefined_local_var() -> None:
with assertRaises(UnboundLocalError, 'local variable "y2" referenced before assignment'):
print(y2)

def test_tuples() -> None:
t1: tuple[float, float] = (1.5, 2.5)
assert t1 == tuple([1.5, 2.5])
n = int() + 5
t2: tuple[float, float, float, float] = (n, 1.5, -7, -113)
assert t2 == tuple([5.0, 1.5, -7.0, -113.0])

[case testFloatGlueMethodsAndInheritance]
from typing import Any
from typing_extensions import Final
Expand Down