Opened 5 years ago

Last modified 18 months ago

#10010 new bug

LLVM/optimized code for sqrt incorrect for negative values

Reported by: glguy Owned by:
Priority: normal Milestone: 8.2.1
Component: Compiler (LLVM) Version: 7.8.4
Keywords: Cc: tjakway, George
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Test Case: libraries/base/tests/Numeric/sqrt
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

The LLVM sqrt intrinsic is undefined below values of -0.0

See http://llvm.org/docs/LangRef.html#llvm-sqrt-intrinsic

In some versions of LLVM it returns 0.0 in this case, but in newer versions the return value is undefined.

This causes the result of Haskell's sqrt to vary at negative values based on the codegen and optimization flags chosen.

main = print (sqrt (-7 :: Double))
$ ghc -O -fllvm Sqrt.hs
[1 of 1] Compiling Main             ( Sqrt.hs, Sqrt.o )
Linking Sqrt ...
$ ./Sqrt 
0.0
$ ghc -O  Sqrt.hs
[1 of 1] Compiling Main             ( Sqrt.hs, Sqrt.o )
Linking Sqrt ...
$ ./Sqrt 
NaN

Change History (8)

comment:1 Changed 3 years ago by tjakway

This works for me on HEAD (f21eedbc223c33b71ba323a44c25fcd512f61b52)

All of the below were tried with -O0, -O1, and -O2

ghc 8.0.1 (doesn't support LLVM 3.8) and llvm 3.7 gives NaN
HEAD with 3.7 also gives NaN
HEAD with 3.8 gives NaN

So at least it's OK going forward.

Last edited 3 years ago by tjakway (previous) (diff)

comment:2 Changed 3 years ago by tjakway

Cc: tjakway added

comment:3 Changed 3 years ago by mpickering

@tjakway, does that mean this is fixed now?

comment:4 in reply to:  3 Changed 3 years ago by tjakway

Replying to mpickering:

@tjakway, does that mean this is fixed now?

I don't remember why I didn't test it with LLVM 3.6 (probably just forgot) but we should probably do that before closing the ticket. I don't have any reason to think 3.6 would be different but just to be sure. Unless we don't support 3.6 with the latest version of GHC, then we can just close it now.

comment:5 Changed 18 months ago by George

This appears to be fixed in ghc 8.4.1 and llvm 6.0. The supported llvm for 8.4.1 is 5.0 but I have 6.0 as I'm on a Mac and did a brew update which overwrote 5.0 and installed 6.0. Can somebody verify with llvm 5.0?

The following shows it works as described above:

bash-3.2$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 8.4.1
bash-3.2$ opt -version
LLVM (http://llvm.org/):
  LLVM version 6.0.0
  Optimized build.
  Default target: x86_64-apple-darwin17.4.0
  Host CPU: nehalem
bash-3.2$ llc -version
LLVM (http://llvm.org/):
  LLVM version 6.0.0
  Optimized build.
  Default target: x86_64-apple-darwin17.4.0
  Host CPU: nehalem
...

 ghc -O -fllvm Sqrt.hs
[1 of 1] Compiling Main             ( Sqrt.hs, Sqrt.o )
You are using an unsupported version of LLVM!
Currently only 5.0 is supported.
We will try though...
Linking Sqrt ...
bash-3.2$ ./Sqrt
NaN
bash-3.2$ 

comment:6 Changed 18 months ago by George

Cc: George added

comment:7 Changed 18 months ago by bgamari

Milestone: 8.2.1
Test Case: libraries/base/tests/Numeric/sqrt

I'm rather surprised that wasn't caught by any existing testcase. Added a test in Phab:D4543.

comment:8 Changed 18 months ago by Ben Gamari <ben@…>

In 5161609/ghc:

testsuite: Add test for negative sqrts (#10010)

Reviewers: hvr, alpmestan

Reviewed By: alpmestan

Subscribers: thomie, carter

GHC Trac Issues: #10010

Differential Revision: https://phabricator.haskell.org/D4543
Note: See TracTickets for help on using tickets.