Skip to content

gccrs: add {add,sub,mul}_with_overflow intrinsics #1945

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

Jump to bottom
Merged
merged 1 commit into from
Mar 1, 2023
Merged

gccrs: add {add,sub,mul}_with_overflow intrinsics #1945

merged 1 commit into from
Mar 1, 2023

Conversation

philberty
Copy link
Member

Fixes #1898

Signed-off-by: Philip Herron [email protected]

gcc/rust/ChangeLog:

* backend/rust-compile-intrinsic.cc (op_with_overflow_inner): (std::function<tree): new wrapper over op_with_overflow (op_with_overflow): generate the intrinsic based on the tree_code op

gcc/testsuite/ChangeLog:

* rust/compile/torture/intrinsics-8.rs: New test.

@philberty philberty requested review from P-E-P and CohenArthur March 1, 2023 11:44
@philberty philberty added the bug label Mar 1, 2023
@philberty philberty added this to the Final upstream patches milestone Mar 1, 2023
@philberty philberty mentioned this pull request Mar 1, 2023
Closed
41 tasks
@philberty
gccrs: add {add,sub,mul}_with_overflow intrinsics
1a8447d 
Fixes #1898

Signed-off-by: Philip Herron <[email protected]>

gcc/rust/ChangeLog:

	* backend/rust-compile-intrinsic.cc (op_with_overflow_inner): wraps op_with_overflow
	(std::function<tree): likewise
	(op_with_overflow): generate the intrinsic based on the tree_code op

gcc/testsuite/ChangeLog:

	* rust/compile/torture/intrinsics-8.rs: New test.
@philberty philberty force-pushed the phil/overflow-intrinsics branch from fa14b2d to 1a8447d Compare March 1, 2023 11:54
CohenArthur
CohenArthur approved these changes Mar 1, 2023
View reviewed changes
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

Nice! LGTM :D It'd be good I think to have a test to make sure the values returned are the same as rustc. Since we have -fwrapv by default I think this should be the case but just to make sure and not introduce regressions later on

P-E-P
P-E-P approved these changes Mar 1, 2023
View reviewed changes
Copy link
Member

@P-E-P P-E-P left a comment

Choose a reason for hiding this comment

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

Looks fine 👍

@philberty philberty added this pull request to the merge queue Mar 1, 2023
Merged via the queue into master with commit 23befb0 Mar 1, 2023
@philberty philberty deleted the phil/overflow-intrinsics branch March 2, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@P-E-P P-E-P P-E-P approved these changes

@CohenArthur CohenArthur CohenArthur approved these changes

Assignees
No one assigned
Labels
bug enhancement
Projects
libcore 1.49
Status: Done
Milestone
Final upstream patches
Development

Successfully merging this pull request may close these issues.

Support overflowing_add in libcore
3 participants
@philberty @P-E-P @CohenArthur