Skip to content
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

math.nan() compares equal to itself #1174

Closed
bnoordhuis opened this Issue Jun 29, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@bnoordhuis
Copy link
Member

bnoordhuis commented Jun 29, 2018

const assert = @import("std").debug.assert;
const math = @import("std").math;

test "nan != nan" {
    assert(math.nan(f32) != math.nan(f32)); // fails because LHS and RHS compare equal
}

This is unlike other languages where NaNs are unequal to everything, including themselves.

Setting FloatMode.Strict or FloatMode.Optimized with @setFloatMode() makes no difference.

@bnoordhuis

This comment has been minimized.

Copy link
Member Author

bnoordhuis commented Jun 29, 2018

I think this is an optimization gone wrong. With --release-small --emit llvm-ir, it reduces to this:

; Function Attrs: nobuiltin nounwind
define internal fastcc i16 @"nan != nan"() unnamed_addr #0 !dbg !620 {
Entry:
  call void @llvm.dbg.value(metadata i1 false, metadata !611, metadata !DIExpression()) #9, !dbg !623
  tail call fastcc void @panic() #9, !dbg !627
  unreachable, !dbg !627
}
@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Jun 29, 2018

Very interesting. It's definitely supposed to evaluate to false when float mode is strict. When float mode is optimized the user gives up ability to use NaNs. So in optimized mode, this operation produces an undefined value. I think we should add a runtime safety check (similar to how we handle integer overflow) for float operations to make sure undefined values don't creep up in code.

@andrewrk andrewrk added this to the 0.3.0 milestone Jun 30, 2018

@andrewrk andrewrk added the bug label Jun 30, 2018

@andrewrk andrewrk modified the milestones: 0.3.0, 0.4.0 Sep 3, 2018

@numsim1415

This comment has been minimized.

Copy link

numsim1415 commented Oct 13, 2018

I just want to point out that this bug (dis-)appears depending on whether the comparison happens during comptime or not.
I tried a few different cases here.

@andrewrk andrewrk closed this in 1dc6751 Apr 5, 2019

@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Apr 5, 2019

it was just the wrong predicate given to LLVM:

--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -1852,7 +1852,7 @@ static LLVMRealPredicate cmp_op_to_real_predicate(IrBinOp cmp_op) {
         case IrBinOpCmpEq:
             return LLVMRealOEQ;
         case IrBinOpCmpNotEq:
-            return LLVMRealONE;
+            return LLVMRealUNE;
         case IrBinOpCmpLessThan:
             return LLVMRealOLT;
         case IrBinOpCmpGreaterThan:

The comptime stuff is now fixed too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.