[GHC] #9238: Negative zero broken
#9238: Negative zero broken --------------------------+------------------------------------------------ Reporter: | Owner: augustss | Status: new Type: bug | Milestone: Priority: normal | Version: 7.8.2 Component: | Operating System: Windows Compiler | Type of failure: Incorrect result at runtime Keywords: | Test Case: Architecture: | Blocking: Unknown/Multiple | Difficulty: | Unknown | Blocked By: | Related Tickets: | --------------------------+------------------------------------------------ Try the following program {{{ compareDouble :: Double -> Double -> Ordering compareDouble x y = case (isNaN x, isNaN y) of (True, True) -> EQ (True, False) -> LT (False, True) -> GT (False, False) -> -- Make -0 less than 0 case (x == 0, y == 0, isNegativeZero x, isNegativeZero y) of (True, True, True, False) -> LT (True, True, False, True) -> GT _ -> x `compare` y main = do let l = [-0, 0] print [ (x, y, compareDouble x y) | x <- l, y <- l ] }}} Compile and run with -O0 {{{ $ ghc -O0 D.hs [1 of 1] Compiling Main ( D.hs, D.o ) Linking D.exe ... $ ./D [(-0.0,-0.0,EQ),(-0.0,0.0,LT),(0.0,-0.0,GT),(0.0,0.0,EQ)] }}} This is the correct output. Compile and run with -O1 {{{ $ ghc -O1 D.hs [1 of 1] Compiling Main ( D.hs, D.o ) Linking D.exe ... $ ./D [(-0.0,-0.0,LT),(-0.0,0.0,LT),(0.0,-0.0,EQ),(0.0,0.0,EQ)] }}} This is wrong. Put a NOINLINE pragma on compareDouble: {{{ $ ghc -O1 D.hs [1 of 1] Compiling Main ( D.hs, D.o ) Linking D.exe ... $ ./D [(-0.0,-0.0,EQ),(-0.0,0.0,EQ),(0.0,-0.0,EQ),(0.0,0.0,EQ)] }}} This is wrong in a different way. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken ------------------------------------------------+-------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Windows | Architecture: Type of failure: Incorrect result at runtime | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: ------------------------------------------------+-------------------------- Comment (by augustss): My guess is that this very function (which gives a total order to most doubles) is needed instead of ordinary comparison somewhere in the ghc optimizer. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken ------------------------------------------------+-------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Windows | Architecture: Type of failure: Incorrect result at runtime | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: ------------------------------------------------+-------------------------- Comment (by ezyang): Appears to be a regression from 7.6. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken ------------------------------------------------+-------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result at runtime | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: ------------------------------------------------+-------------------------- Changes (by ezyang): * os: Windows => Unknown/Multiple -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken ------------------------------------------------+-------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result at runtime | Unknown/Multiple Test Case: | Difficulty: Blocking: | Unknown | Blocked By: | Related Tickets: ------------------------------------------------+-------------------------- Comment (by carter): I can reproduce the behavior on ghc 7.8.2 on OS X 64 bit i've also started taking a look at the core generated for the 3 alternatives lennart laid out, using variations of the following ghc invocation {{{ ghc -O1 -ddump-simpl -dsuppress-idinfo -dsuppress-coercions -dsuppress- type-applications -dsuppress-uniques -dsuppress-module-prefixes bugMain.hs
bugO1CoreNOINLINE.simple }}}
though i'm still working through which differences are real and which arent https://gist.github.com/cartazio/351f15ae74257a7ab64d -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: | Architecture: Unknown/Multiple Unknown/Multiple | Difficulty: Unknown Type of failure: Incorrect | Blocked By: result at runtime | Related Tickets: Test Case: | Blocking: | Differential Revisions: | -------------------------------------+------------------------------------- Comment (by rwbarton): When we do {{{ case ds1 of wild_XT { -- ds1 :: Double# (y = D# ds1) __DEFAULT -> ... 0.0 -> ... } }}} inside the `0.0` case we seem to create a local unfolding `ds1 = 0.0`. You can see this happening with `-dverbose-core2core -ddump-inlinings`: search for "Inlining done: x" or "ds1". This is wrong because the `0.0` case matches both positive and negative zero. That is the correct semantics that makes the Core match the original program, and it is implemented correctly by the code generation. But it means that we can't create this local unfolding when the pattern is specifically `0.0`. I don't know where this unfolding gets created, or I'd try to fix it myself. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: | Architecture: Unknown/Multiple Unknown/Multiple | Difficulty: Unknown Type of failure: Incorrect | Blocked By: result at runtime | Related Tickets: Test Case: | Blocking: | Differential Revisions: | -------------------------------------+------------------------------------- Comment (by simonpj): I can tell you exactly where it's created: line 2107 of `Simplify.lhs` (in today's HEAD), in function `simplAlt`. However I'm suspicious of adding a special case for float/double here. Rather, I think we should prohibit using Core-language `case` expressions to scrutinise float/double, so that `case` (in Core) behaves in a simple, predictable way. Rather I think we should probably generate {{{ case eqDouble# ds1 0.0## of True -> ... False -> ... }}} (or, rather, today's unboxed-boolean version). Now the magic is confined to how `eqDouble#` is implemented, which is the proper place for it. Now Haskell source code does allow case expressions over floats, but that's just a question of fixing the desugarer. Does that make sense? Does anyone feel like taking this on? Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: | Architecture: Unknown/Multiple Unknown/Multiple | Difficulty: Unknown Type of failure: Incorrect | Blocked By: result at runtime | Related Tickets: #7858 Test Case: | Blocking: | Differential Revisions: | -------------------------------------+------------------------------------- Changes (by rwbarton): * related: => #7858 Comment: This arose also in #7858. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: 7.8.4 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: | Architecture: Unknown/Multiple Unknown/Multiple | Difficulty: Unknown Type of failure: Incorrect | Blocked By: result at runtime | Related Tickets: #7858, #9451 Test Case: | Blocking: | Differential Revisions: | -------------------------------------+------------------------------------- Changes (by rwbarton): * related: #7858 => #7858, #9451 * milestone: => 7.8.4 Comment: And #9451. Ideally we would fix this for 7.8.4 since it is a regression from 7.6.3. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: 7.10.1 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: | Architecture: Unknown/Multiple Unknown/Multiple | Difficulty: Unknown Type of failure: Incorrect | Blocked By: result at runtime | Related Tickets: #7858, #9451 Test Case: | Blocking: | Differential Revisions: | -------------------------------------+------------------------------------- Description changed by thomie: Old description:
Try the following program {{{ compareDouble :: Double -> Double -> Ordering compareDouble x y = case (isNaN x, isNaN y) of (True, True) -> EQ (True, False) -> LT (False, True) -> GT (False, False) -> -- Make -0 less than 0 case (x == 0, y == 0, isNegativeZero x, isNegativeZero y) of (True, True, True, False) -> LT (True, True, False, True) -> GT _ -> x `compare` y
main = do let l = [-0, 0] print [ (x, y, compareDouble x y) | x <- l, y <- l ] }}}
Compile and run with -O0 {{{ $ ghc -O0 D.hs [1 of 1] Compiling Main ( D.hs, D.o ) Linking D.exe ... $ ./D [(-0.0,-0.0,EQ),(-0.0,0.0,LT),(0.0,-0.0,GT),(0.0,0.0,EQ)] }}} This is the correct output.
Compile and run with -O1 {{{ $ ghc -O1 D.hs [1 of 1] Compiling Main ( D.hs, D.o ) Linking D.exe ... $ ./D [(-0.0,-0.0,LT),(-0.0,0.0,LT),(0.0,-0.0,EQ),(0.0,0.0,EQ)] }}} This is wrong.
Put a NOINLINE pragma on compareDouble: {{{ $ ghc -O1 D.hs [1 of 1] Compiling Main ( D.hs, D.o ) Linking D.exe ... $ ./D [(-0.0,-0.0,EQ),(-0.0,0.0,EQ),(0.0,-0.0,EQ),(0.0,0.0,EQ)] }}} This is wrong in a different way.
New description: Try the following program {{{ compareDouble :: Double -> Double -> Ordering compareDouble x y = case (isNaN x, isNaN y) of (True, True) -> EQ (True, False) -> LT (False, True) -> GT (False, False) -> -- Make -0 less than 0 case (x == 0, y == 0, isNegativeZero x, isNegativeZero y) of (True, True, True, False) -> LT (True, True, False, True) -> GT _ -> x `compare` y main = do let l = [-0, 0] print [ (x, y, compareDouble x y) | x <- l, y <- l ] }}} Compile and run with -O0 {{{ $ ghc -O0 -fforce-recomp D.hs [1 of 1] Compiling Main ( D.hs, D.o ) Linking D.exe ... $ ./D [(-0.0,-0.0,EQ),(-0.0,0.0,LT),(0.0,-0.0,GT),(0.0,0.0,EQ)] }}} This is the correct output. Compile and run with -O1 {{{ $ ghc -O1 -fforce-recomp D.hs [1 of 1] Compiling Main ( D.hs, D.o ) Linking D.exe ... $ ./D [(-0.0,-0.0,LT),(-0.0,0.0,LT),(0.0,-0.0,EQ),(0.0,0.0,EQ)] }}} This is wrong. Put a NOINLINE pragma on compareDouble: {{{ $ ghc -O1 -fforce-recomp D.hs [1 of 1] Compiling Main ( D.hs, D.o ) Linking D.exe ... $ ./D [(-0.0,-0.0,EQ),(-0.0,0.0,EQ),(0.0,-0.0,EQ),(0.0,0.0,EQ)] }}} This is wrong in a different way. -- -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #7858, #9451 | Differential Revisions: -------------------------------------+------------------------------------- Comment (by lerkok): Similar issue, and perhaps exactly the same bug: https://ghc.haskell.org/trac/ghc/ticket/10215#ticket -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
I can tell you exactly where it's created: line 2107 of `Simplify.lhs` (in today's HEAD), in function `simplAlt`.
However I'm suspicious of adding a special case for float/double here. Rather, I think we should prohibit using Core-language `case` expressions to scrutinise float/double, so that `case` (in Core) behaves in a simple,
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #7858, #9451 | Differential Revisions: -------------------------------------+------------------------------------- Comment (by bgamari): Replying to [comment:6 simonpj]: predictable way.
Rather I think we should probably generate {{{ case eqDouble# ds1 0.0## of True -> ... False -> ... }}} (or, rather, today's unboxed-boolean version). Now the magic is
confined to how `eqDouble#` is implemented, which is the proper place for it. simonpj, how do you think your suggestion should interact with the `litEq` rule? As far as I can tell this `PrelRule` is responsible for most of the floating-point pattern matches that we are seeing. As you essentially suggesting we disable `litEq` for floating-point types? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #7858, #9451 | Differential Revisions: -------------------------------------+------------------------------------- Comment (by simonpj): Replying to [comment:13 bgamari]:
simonpj, how do you think your suggestion should interact with the `litEq` rule? As far as I can tell this `PrelRule` is responsible for most of the floating-point pattern matches that we are seeing. As you essentially suggesting we disable `litEq` for floating-point types?
Well, IF we agree that Core case expressions should not scrutinise floating point values, THEN * We should document that invariant, with explanation of why * We should make Core Lint check for it * And yes `litEq` (which generates such case expressions) should not do so for floating point values Simon -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:14 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: 7.12.1 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #7858, #9451 | Differential Revisions: Phab:D1061 -------------------------------------+------------------------------------- Changes (by bgamari): * differential: => Phab:D1061 -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:15 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: new Priority: normal | Milestone: 7.10.3 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #7858, #9451, | Differential Revisions: Phab:D1061 #10215 | -------------------------------------+------------------------------------- Changes (by YitzGale): * cc: gale@… (added) * related: #7858, #9451 => #7858, #9451, #10215 * milestone: 8.0.1 => 7.10.3 Comment: As per Austin's request on ghc-dev, changing milestone to 7.10.3 due to: https://github.com/LeventErkok/sbv/issues/138 It appears from the submission that Lennart also needs this. And anyway this looks like a regression that really should be fixed. However, I do note that this issue doesn't have a patch yet. So if this is wrong, I apologize. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:17 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken
-------------------------------------+-------------------------------------
Reporter: augustss | Owner:
Type: bug | Status: new
Priority: normal | Milestone: 7.10.3
Component: Compiler | Version: 7.8.2
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
Type of failure: Incorrect result | Unknown/Multiple
at runtime | Test Case:
Blocked By: | Blocking:
Related Tickets: #7858, #9451, | Differential Rev(s): Phab:D1061
#10215 |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: merge Priority: normal | Milestone: 7.10.3 Component: Compiler | Version: 7.8.2 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #7858, #9451, | Differential Rev(s): Phab:D1061 #10215 | -------------------------------------+------------------------------------- Changes (by bgamari): * status: new => merge Comment: Fixed. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:19 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#9238: Negative zero broken -------------------------------------+------------------------------------- Reporter: augustss | Owner: Type: bug | Status: closed Priority: normal | Milestone: 7.10.3 Component: Compiler | Version: 7.8.2 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: #7858, #9451, | Differential Rev(s): Phab:D1061 #10215 | -------------------------------------+------------------------------------- Changes (by bgamari): * status: merge => closed * resolution: => fixed Comment: Merged to `ghc-7.10` as 293bf83f209ed6202a131456bf93fd472a790b13. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/9238#comment:20 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC