Opened 7 years ago

Closed 5 years ago

#7460 closed bug (fixed)

Double literals generated bad core

Reported by: tibbe Owned by: tibbe
Priority: normal Milestone: 7.10.1
Component: Compiler Version: 7.4.2
Keywords: Cc: johan.tibell@…, daniel.is.fischer@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

The following code results in core containing expression like doubleFromInteger (wordToInteger sc_s2pw):

{-# LANGUAGE CPP, BangPatterns #-}
module Main (main) where

#define VDIM 100
#define VNUM 100000

import Data.Array.Base
import Data.Array.ST
import Data.Array.Unboxed
import Control.Monad.ST
import GHC.Word
import Control.Monad
import Data.Bits

prng :: Word -> Word
prng w = w'
  where
    !w1 = w `xor` (w `shiftL` 13)
    !w2 = w1 `xor` (w1 `shiftR` 7)
    !w' = w2 `xor` (w2 `shiftL` 17)

type Vec s = STUArray s Int Double

kahan :: Vec s -> Vec s -> ST s ()
kahan s c = do
    let inner w j
            | j < VDIM  = do
                !cj <- unsafeRead c j
                !sj <- unsafeRead s j
                let !y = fromIntegral w - cj
                    !t = sj + y
                    !w' = prng w
                unsafeWrite c j ((t-sj)-y)
                unsafeWrite s j t
                inner w' (j+1)
            | otherwise = return ()
        outer i | i < VNUM = inner i 0 >> outer (i + 1)
                | otherwise = return ()
    outer 0

calc :: ST s (Vec s)
calc = do
    s <- newArray (0,VDIM-1) 0
    c <- newArray (0,VDIM-1) 0
    kahan s c
    return s

main :: IO ()
main = print . elems $ runSTUArray calc

Change History (15)

comment:1 Changed 7 years ago by igloo

difficulty: Unknown
[20:48] < tibbe> Igloo: is the fix for converting small integers to doubles in 
                 7.4.2? http://hackage.haskell.org/trac/ghc/ticket/6111
[20:49] < tibbe> Igloo: I'm currently on 7.4.2 and I'm getting 
                 doubleFromInteger (wordToInteger sc_s2pw) in my code

comment:2 Changed 7 years ago by tibbe

Changing the use of fromIntegral to

word2Double x = D# (int2Double# (word2Int# x))

works around the issue in my case.

comment:3 in reply to:  2 Changed 7 years ago by tibbe

Replying to tibbe:

Changing the use of fromIntegral to

word2Double x = D# (int2Double# (word2Int# x))

works around the issue in my case.

Except that it doesn't always generate the correct value. The point remains though, we need an efficient word2Double conversion to be inserted into the core.

comment:4 Changed 7 years ago by daniel.is.fischer

Cc: daniel.is.fischer@… added

While we have no real word2Double#, would

{-# RULES
"fromIntegral/Word->Double" fromIntegral = word2Double
  #-}

correction :: Double
correction = 2 * int2Double minBound

word2Double :: Word -> Double
word2Double w = case fromIntegral w of
                    i | i < 0 -> int2Double i - correction
                      | otherwise -> int2Double i

(and similar for Float) be an acceptable stand-in?

comment:5 Changed 7 years ago by simonpj

Johan, can you drive this ticket to conclusion?

Is the Right Thing to implement a word2Double# primitive? And word2Float# I guess. What exactly does it do? Just treat the word as a unsigned integer, thus behaving just like int2Double# (at least as I understand it)?

If so why not do precisely that, rather than use workarounds? Presuambly the code generators would need to be educated.

Simon

comment:6 Changed 7 years ago by tibbe

Simon, I don't think we need new primitives, as this is a regression and hence it used to work without extra primitives. I'm not quite sure why it regressed. Perhaps it's due to some refactoring that touches inling. Ian, do you have any idea why this regressed?

comment:7 Changed 7 years ago by igloo

What's the regression exactly? In 7.0 I see

              case GHC.Prim.>=# i#_a1x8 0 of _ {
                GHC.Bool.False ->
                  case {__pkg_ccall_GC integer-gmp integer_cmm_word2Integerzh GHC.Prim.Word#
                                                       -> (# GHC.Prim.Int#, GHC.Prim.ByteArray# #)}_a1yc
                         x#_a16l
                  of _ { (# s1_a1y9, d_a1ya #) ->
                  case {__pkg_ccall integer-gmp integer_cbits_encodeDouble GHC.Prim.Int#
                                                    -> GHC.Prim.ByteArray#
                                                    -> GHC.Prim.Int#
                                                    -> GHC.Prim.State# GHC.Prim.RealWorld
                                                    -> (# GHC.Prim.State# GHC.Prim.RealWorld,
                                                          GHC.Prim.Double# #)}_a1yl
                         s1_a1y9 d_a1ya 0 GHC.Prim.realWorld#

as opposed to

              case GHC.Prim.>=# i#_a1F4 0 of _ {
                GHC.Types.False ->
                  case GHC.Integer.Type.doubleFromInteger
                         (GHC.Integer.Type.wordToInteger x#_a1iv)

in 7.4. Is that change what you are talking about, or are you looking at something different?

comment:8 Changed 7 years ago by tibbe

This came up as we investigated the performance of the above program under 7.4. int-e reports the following run-times:

int-e: so ghc-7.0.3 has calls to integer_cmm_word2Integerzh and integer_cbits_encodeDouble instead of doubleFromInteger (wordToInteger ...), so what is the difference?
[10:15am] int-e: tibbe: I get http://int-e.cohomology.org/~bf3/haskell/doubles-7.0.3.core and http://int-e.cohomology.org/~bf3/haskell/doubles-7.4.1.core both with -O2 -ddump-simpl
[10:17am] tibbe: int-e: I wonder what cmm integer_cbits_encodeDouble generates
[10:17am] tibbe: int-e: whatever it does it makes a big difference to performance
[10:27am] int-e: hmm, how big? I have 0.28s from 7.0.3, 0.27 from 7.2.1 and 0.34s from 7.4.1. 0.3 from 7.6.1. Similar for -O1.
[10:28am] int-e: (x86_64)
[10:31am] int-e: x86 is more pronounced. 0m1.903s / 0m1.794s / 0m2.511s / 0m2.046s for 7.0.3, 7.2.1, 7.4.1, 7.6.1
[10:31am] int-e: (-O1)
[10:31am] tibbe: int-e: so 20% on x86-64
[10:31am] int-e: (and atom)

comment:9 Changed 7 years ago by int-e

Looking at the CMM code, the most noticable difference seems that ghc-7.0.3 used to generate a foreign call for the second part of the conversion,

          integer_cbits_encodeDouble((_c2ut::I64, `signed'),
                                     (_c2uv::I64, PtrHint), (0, `signed'))[_unsafe_call_];

while the current code (since ghc-7.2.1) sets up a stack frame and jumps into the integer package.

Not sure how much of the observed differences that explains.

comment:10 Changed 7 years ago by tibbe

Even if 7.0 was a bit better it seems like having a word2Double# and a word2Float# primop makes sense. They should behave in the same way as fromIntegral (w :: Word) :: Double and fromIntegral (w :: Word) :: Float does today presumably.

comment:11 Changed 7 years ago by tibbe

This is how the C standard defines integer to/from floating point conversions:

6.3.1.4 Real floating and integer

1 When a finite value of real floating type is converted to an integer type other than _Bool, the fractional part is discarded (i.e., the value is truncated toward zero). If the value of the integral part cannot be represented by the integer type, the behavior is undefined. 61)

2 When a value of integer type is converted to a real floating type, if the value being converted can be represented exactly in the new type, it is unchanged. If the value being converted is in the range of values that can be represented but cannot be represented exactly, the result is either the nearest higher or nearest lower representable value, chosen in an implementation-defined manner. If the value being converted is outside the range of values that can be represented, the behavior is undefined. Results of some implicit conversions may be represented in greater range and precision than that required by the new type (see 6.3.1.8 and 6.8.6.4).

61) The remaindering operation performed when a value of integer type is converted to unsigned type need not be performed when a value of real floating type is converted to unsigned type. Thus, the range of portable real floating values is (−1, Utype_MAX+1).

comment:12 Changed 7 years ago by tibbe

Owner: set to tibbe

I will try to add the needed primops.

comment:13 Changed 6 years ago by igloo

Milestone: 7.8.1

comment:14 Changed 5 years ago by thoughtpolice

Milestone: 7.8.37.10.1

Moving to 7.10.1

comment:15 Changed 5 years ago by thomie

Resolution: fixed
Status: newclosed

The word2Float and word2Double primops were added, which solved this issue. The output of ghc-7.6.3 -ddump-simpl -O2 test.hs contained calls to doubleFromInteger, but the same command using ghc-7.8.3 does not (which is good). Other things must have changed as well, because the code is about twice as fast with 7.8.3, while comment:8 talked about a difference of only 20%.

commit 2e8c769422740c001e0a247bfec61d4f78598582

Author: Johan Tibell <>
Date:   Wed Dec 5 19:08:48 2012 -0800

    Implement word2Float# and word2Double#

commit cd01e48fbc548ff8d81ab547108bfdde8a113cd7

Author: Johan Tibell <>
Date:   Thu Dec 13 12:03:40 2012 -0800

    Add test for word2Double# and word2Float#

commit a18cf9cbdfec08732f5b7e0c886a5d899a6a5998

Author: Johan Tibell <>
Date:   Thu Dec 13 14:49:58 2012 -0800

    Add fromIntegral/Word->Double and fromIntegral/Word-Float rules

commit 8cd4ced57dccc1f4f54d242982209ec61e145700

Author: Johan Tibell <>
Date:   Tue Dec 18 14:40:02 2012 +0100

    perf test for Word->Float/Double conversion

commit 6d5f25f5e0b33173fb2e7983cab40808c723f220

Author: Geoffrey Mainland <>
Date:   Thu Jan 3 16:59:03 2013 +0000

    Fix LLVM code generated for word2Float# and word2Double#.

commit 744035fdd4b882c17ef7c6e4439b9e7099e7ec3d

Author: Johan Tibell <>
Date:   Mon Jan 7 21:35:07 2013 -0800

    Fix Word2Float# test on 32-bit
Note: See TracTickets for help on using tickets.