Skip to content

s390x: document the different rounding flavors #1869

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.

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

Merged
merged 1 commit into from
Jul 15, 2025

Conversation

folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Jul 12, 2025

If I remember right, these all function equivalent on the target (because the rounding mode can't be changed). At least on CE they generate the same instructions.

cc @uweigand

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2025

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@uweigand
Copy link

This doesn't look right. We certainly can change the default rounding mode on s390x just like on most other targets; the rint and nearbyint functions respect the current rounding mode. It is true that clang does make a default assumption (on all targets) that the default rounding mode is set to roundeven; but that assumption can be changed by using the -frounding-math command line option. (Also, even this assumption only affects potential compile-time optimizations made by LLVM; the emitted code will in any case use different instructions that will / will not respect current rounding mode at the ISA level.)

In addition, rint and nearbyint differ in whether or not they will generate the inexact floating-point exception. Again, by default clang makes the assumption that (any) floating-point exception will have no effect and can be ignored; but that assumption can be changed using the -ffp-exception-behavior= option.

Here's a full table of the various nearest-integer functions in IBM Z vector intrinsics, C23 math.h, LLVM IR, the equivalent ISO/IEC operation, and the corresponding set of vfidb instruction parameters:

Z vector intrinsic      C23 math.h  LLVM IR         ISO/IEC 60559 operation        inexact  vfidb parameters
vec_rint                rint        llvm.rint       roundToIntegralExact           yes      0, 0        
vec_roundc              nearbyint   llvm.nearbyint  n/a                            no       4, 0       
vec_floor / vec_roundm  floor       llvm.floor      roundToIntegralTowardNegative  no       4, 7       
vec_ceil / vec_roundp   ceil        llvm.ceil       roundToIntegralTowardPositive  no       4, 6       
vec_trunc / vec_roundz  trunc       llvm.trunc      roundToIntegralTowardZero      no       4, 5       
vec_round               roundeven   llvm.roundeven  roundToIntegralTiesToEven      no       4, 4       
n/a                     round       llvm.round      roundToIntegralTiesAway        no       4, 1       

@folkertdev folkertdev force-pushed the s390x-use-rounding-intrinsic branch from f6414e7 to 9f127ae Compare July 14, 2025 15:22
@folkertdev
Copy link
Contributor Author

Ah, right, we can't actually test for the vfidb parameters, so CI is OK with such changes.

Also, simd_round_ties_even is implemented as llvm.rint: https://github.com/sayantn/rust/blob/2ffa1dd392c1fe9c23a2f11f7b7a293a744c05ce/compiler/rustc_codegen_gcc/src/intrinsic/simd.rs#L782

I guess the name is confusing no matter what, but simd_round_ties_even not using llvm.roundeven is unexpected.

Anyway, I copied that (very helpful!) table as a comment, so hopefully there will not be any confusion in the future.

Also the rust llvm 21 update is close, so then we can resolve that TODO.

@folkertdev folkertdev changed the title s390x: use simd_round_ties_even in more places s390x: document the different rounding flavors Jul 14, 2025
@Amanieu Amanieu added this pull request to the merge queue Jul 14, 2025
Merged via the queue into rust-lang:master with commit d22c313 Jul 15, 2025
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants