-
Notifications
You must be signed in to change notification settings - Fork 13.5k
InstSimplify folds rint() and nearby() ignoring the run-time rounding mode #53809
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
Comments
I think this is working as intended: |
#include <fenv.h>
#include <math.h>
#include <stdio.h>
int main() {
fesetround(FE_TONEAREST);
printf("%lf\n", nearbyint(1.25));
fesetround(FE_UPWARD);
printf("%lf\n", nearbyint(1.25));
return 0;
}
Now with clang: define dso_local i32 @main() #0 {
entry:
%retval = alloca i32, align 4
store i32 0, i32* %retval, align 4
%call = call i32 @fesetround(i32 noundef 0) #4
%0 = call double @llvm.nearbyint.f64(double 1.250000e+00)
%call1 = call i32 (i8*, ...) @printf(i8* noundef getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0), double noundef %0)
%call2 = call i32 @fesetround(i32 noundef 2048) #4
%1 = call double @llvm.nearbyint.f64(double 1.250000e+00)
%call3 = call i32 (i8*, ...) @printf(i8* noundef getelementptr inbounds ([5 x i8], [5 x i8]* @.str, i64 0, i64 0), double noundef %1)
ret i32 0
} So clang gives different results with -O0 and -O1. |
oh I see, if FENV_ACCESS is off I should ignore the man pages about rounding. |
I strongly believe that to be the spirit of what clang/llvm provides/intents to provide/guarantee, if not the letter, yes. |
I wish we could link bugs in a list like bugzilla did . |
Test
llvm/test/Transforms/InstSimplify/ConstProp/rint.ll
is very wrong for what I can tell.The man page of nearbyint/rint says that these functions use the rounding mode as set by
fesetround
, but InstSimplify assumes the LLVM default rounding mode.Example:
Depending on the rounding mode, this function returns 1.0 or 2.0. We can't blindly fold these AFAIU.
cc @LebedevRI @rotateright @fhahn
The text was updated successfully, but these errors were encountered: