From 87deb92adc636425a3597f696dc4a251aa920b1a Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 20 Sep 2021 16:46:35 +0100 Subject: [PATCH] [mypyc] Fix division of negative tagged int 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. --- mypyc/lib-rt/int_ops.c | 11 ++++++----- mypyc/test-data/fixtures/ir.py | 3 ++- mypyc/test-data/run-integers.test | 33 ++++++++++++++++++++++--------- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/mypyc/lib-rt/int_ops.c b/mypyc/lib-rt/int_ops.c index 0ac9b6fe490b7..1275f2c105776 100644 --- a/mypyc/lib-rt/int_ops.c +++ b/mypyc/lib-rt/int_ops.c @@ -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); diff --git a/mypyc/test-data/fixtures/ir.py b/mypyc/test-data/fixtures/ir.py index 362495fdaa683..5fdf0b2035232 100644 --- a/mypyc/test-data/fixtures/ir.py +++ b/mypyc/test-data/fixtures/ir.py @@ -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]]: ... diff --git a/mypyc/test-data/run-integers.test b/mypyc/test-data/run-integers.test index 59e61931439bf..4106b9d7191c1 100644 --- a/mypyc/test-data/run-integers.test +++ b/mypyc/test-data/run-integers.test @@ -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 @@ -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 @@ -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): @@ -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)