Skip to content

Commit

Permalink
[mypyc] Fix division of negative tagged int (#11168)
Browse files Browse the repository at this point in the history
The rounding was incorrect for some negative values, because of a bug
related to the tagged integer logic. For example, -5 // 2 evaluated to
-4 instead of -3 (ouch). Change the rounding implementation to not use
shifted integers.

Also add more exhaustive testing and fix a broken integer test case.
  • Loading branch information
JukkaL authored Sep 22, 2021
1 parent 58fb493 commit 2f2c377
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
11 changes: 6 additions & 5 deletions mypyc/lib-rt/int_ops.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,16 +190,17 @@ CPyTagged CPyTagged_Multiply(CPyTagged left, CPyTagged right) {
}

CPyTagged CPyTagged_FloorDivide(CPyTagged left, CPyTagged right) {
if (CPyTagged_CheckShort(left) && CPyTagged_CheckShort(right)
if (CPyTagged_CheckShort(left)
&& CPyTagged_CheckShort(right)
&& !CPyTagged_MaybeFloorDivideFault(left, right)) {
Py_ssize_t result = ((Py_ssize_t)left / CPyTagged_ShortAsSsize_t(right)) & ~1;
Py_ssize_t result = CPyTagged_ShortAsSsize_t(left) / CPyTagged_ShortAsSsize_t(right);
if (((Py_ssize_t)left < 0) != (((Py_ssize_t)right) < 0)) {
if (result / 2 * right != left) {
if (result * right != left) {
// Round down
result -= 2;
result--;
}
}
return result;
return result << 1;
}
PyObject *left_obj = CPyTagged_AsObject(left);
PyObject *right_obj = CPyTagged_AsObject(right);
Expand Down
3 changes: 2 additions & 1 deletion mypyc/test-data/fixtures/ir.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,8 @@ def next(i: Iterator[T]) -> T: pass
def next(i: Iterator[T], default: T) -> T: pass
def hash(o: object) -> int: ...
def globals() -> Dict[str, Any]: ...
def setattr(object: Any, name: str, value: Any) -> None: ...
def getattr(obj: object, name: str) -> Any: ...
def setattr(obj: object, name: str, value: Any) -> None: ...
def enumerate(x: Iterable[T]) -> Iterator[Tuple[int, T]]: ...
@overload
def zip(x: Iterable[T], y: Iterable[S]) -> Iterator[Tuple[T, S]]: ...
Expand Down
33 changes: 24 additions & 9 deletions mypyc/test-data/run-integers.test
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@ assert neg(-9223372036854775807) == 9223372036854775807
assert neg(9223372036854775808) == -9223372036854775808
assert neg(-9223372036854775808) == 9223372036854775808

[case testIsinstanceIntAndNotBool]
def test_isinstance_int_and_not_bool(value: object) -> bool:
return isinstance(value, int) and not isinstance(value, bool)
[file driver.py]
from native import test_isinstance_int_and_not_bool
assert test_isinstance_int_and_not_bool(True) == False
assert test_isinstance_int_and_not_bool(1) == True

[case testIntOps]
def check_and(x: int, y: int) -> None:
# eval() can be trusted to calculate expected result
Expand All @@ -157,15 +165,6 @@ def check_bitwise(x: int, y: int) -> None:
check_or(ll, rr)
check_xor(ll, rr)

[case testIsinstanceIntAndNotBool]
def test_isinstance_int_and_not_bool(value: object) -> bool:
return isinstance(value, int) and not isinstance(value, bool)
[file driver.py]
from native import test_isinstance_int_and_not_bool
assert test_isinstance_int_and_not_bool(True) == False
assert test_isinstance_int_and_not_bool(1) == True


SHIFT = 30
DIGIT0a = 615729753
DIGIT0b = 832796681
Expand Down Expand Up @@ -198,6 +197,10 @@ def test_and_or_xor() -> None:
check_bitwise(BIG_SHORT, DIGIT0a + DIGIT1a + DIGIT2a)
check_bitwise(BIG_SHORT, DIGIT0a + DIGIT1a + DIGIT2a + DIGIT50)

for x in range(-25, 25):
for y in range(-25, 25):
check_bitwise(x, y)

def test_bitwise_inplace() -> None:
# Basic sanity checks; these should use the same code as the non-in-place variants
for x, y in (DIGIT0a, DIGIT1a), (DIGIT2a, DIGIT0a + DIGIT2b):
Expand Down Expand Up @@ -328,3 +331,15 @@ def test_int_as_bool() -> None:
for x in 1, 55, -1, -7, 1 << 50, 1 << 101, -(1 << 50), -(1 << 101):
assert is_true(x)
assert not is_false(x)

def test_divide() -> None:
for x in range(-100, 100):
for y in range(-100, 100):
if y != 0:
assert x // y == getattr(x, "__floordiv__")(y)

def test_mod() -> None:
for x in range(-100, 100):
for y in range(-100, 100):
if y != 0:
assert x % y == getattr(x, "__mod__")(y)

0 comments on commit 2f2c377

Please sign in to comment.