float.__mod__ and float.__divmod__ produce incorrect results for negative numbers#777
Open
pauvrepetit wants to merge 2 commits intoexaloop:developfrom
Open
float.__mod__ and float.__divmod__ produce incorrect results for negative numbers#777pauvrepetit wants to merge 2 commits intoexaloop:developfrom
pauvrepetit wants to merge 2 commits intoexaloop:developfrom
Conversation
The current __mod__ (using LLVM frem) and __floordiv__ implementations have mismatched semantics. When dealing with negative numbers, this leads to (a // b) * b + (a % b) != a, which is clearly a bug. LLVM does not provide an instruction that directly corresponds to Python's floored mod for floats. Since __floordiv__ is already implemented as floor(a / b), computing __mod__ via the identity (a // b) * b + (a % b) == a, i.e. a - (a // b) * b, is a better and more correct approach. Accordingly, __divmod__ which relied on the frem-based __mod__ logic is also updated to match. Affected types: float, float32, float16, bfloat16, float128.
… type Delegate to float(self) % other and float(self) // other instead of using LLVM instructions directly, ensuring consistent semantics with the corrected float.__mod__ and float.__floordiv__ implementations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The current
__mod__(using LLVM frem) and__floordiv__implementations have mismatched semantics. When dealing with negative numbers, this leads to(a // b) * b + (a % b) != a, which is clearly a bug.LLVM does not provide an instruction that directly corresponds to Python's floored mod for floats. Since
__floordiv__is already implemented asfloor(a / b), computing__mod__via the identity(a // b) * b + (a % b) == a, i.e.a - (a // b) * b, is a better and more correct approach.Accordingly,
__divmod__which relied on the frem-based__mod__logic is also updated to match.Affected types: float, float32, float16, bfloat16, float128.
Fixing this issue #775