I recently noticed that with -O1, ghc was optimizing some code if Trace.traceShowId "b" $ True then return $ Left $ Serialize.BadMagic (Serialize.magicBytes magic) file_magic else first Serialize.UnserializeError <$> Exception.evaluate (Serialize.decode rest) to always evaluate the 'else' branch. This is fixed in 8.4.2, so I'm pretty sure it's https://ghc.haskell.org/trac/ghc/ticket/13930 I assume there's no point trying to get a minimal reproduction, since it's a known issue and fixed. Still, https://downloads.haskell.org/~ghc/8.4.2/docs/html/users_guide/8.4.2-notes.h... says "incorrectly optimised, resulting in space leaks". Maybe it should instead say "incorrectly optimised, resulting in taking the wrong branch in 'if' expressions"? That's a bit more alarming, and is a stronger "upgrade to 8.4.2" signal.
Evan Laforge
I recently noticed that with -O1, ghc was optimizing some code
if Trace.traceShowId "b" $ True then return $ Left $ Serialize.BadMagic (Serialize.magicBytes magic) file_magic else first Serialize.UnserializeError <$> Exception.evaluate (Serialize.decode rest)
to always evaluate the 'else' branch. This is fixed in 8.4.2, so I'm pretty sure it's https://ghc.haskell.org/trac/ghc/ticket/13930
I assume there's no point trying to get a minimal reproduction, since it's a known issue and fixed. Still, https://downloads.haskell.org/~ghc/8.4.2/docs/html/users_guide/8.4.2-notes.h... says "incorrectly optimised, resulting in space leaks".
Maybe it should instead say "incorrectly optimised, resulting in taking the wrong branch in 'if' expressions"? That's a bit more alarming, and is a stronger "upgrade to 8.4.2" signal.
Yes, I suppose the language in the release notes does rather understate the degree of the incorrectness. I'll push a new version of the manual with some stronger language tomorrow. Cheers, - Ben
| I recently noticed that with -O1, ghc was optimizing some code
|
| if Trace.traceShowId "b" $ True
| then ...
| else ...
|
| to always evaluate the 'else' branch.
Are you certain this is #13930? I don't see an obvious connection. It seems really really terrible to "optimise" True to False!
I think #13930 was fixed by #5129, which in turn was about discarding a call to 'evaluate'. That is different to turning True to False.
But there's probably some more complicated context to your use-case that means my understanding is faulty.
If you are confident that it's securely fixed, well and good. But when bugs disappear I always worry that they are still there, just concealed by some other change.
Simon
| -----Original Message-----
| From: ghc-devs
On Wed, May 2, 2018 at 1:24 AM, Simon Peyton Jones
| I recently noticed that with -O1, ghc was optimizing some code | | if Trace.traceShowId "b" $ True | then ... | else ... | | to always evaluate the 'else' branch.
Are you certain this is #13930? I don't see an obvious connection. It seems really really terrible to "optimise" True to False!
I think #13930 was fixed by #5129, which in turn was about discarding a call to 'evaluate'. That is different to turning True to False.
But there's probably some more complicated context to your use-case that means my understanding is faulty.
If you are confident that it's securely fixed, well and good. But when bugs disappear I always worry that they are still there, just concealed by some other change.
I'm not totally confident, which is I why I asked. It does seem to be related to the presence of Exception.evaluate, but it also comes and goes depending on how many things are in the condition and branches. It does seem to be gone in 8.4.1, but I'm also a bit nervous when I don't know exactly why something was fixed. I'll try to reduce this to as small an expression as possible that still triggers True -> False.
Ok, here's a short module: import qualified Control.Exception as Exception main :: IO () main = do unserialize putStrLn "all is well" unserialize :: IO Char unserialize = if definitelyTrue then do return 'a' else do Exception.evaluate (error "wrong place") {-# NOINLINE definitelyTrue #-} definitelyTrue :: Bool definitelyTrue = True When compiled with -O on 8.4.1, this should print "wrong place". Without -O, or with 8.4.2, or if True can be inlined, or without evaluate, all is well.
Wow. Could you open a ticket?
I just tried with 8.2.2 which is what I have on this laptop, but it printed "all is well". Does that mean it was fine in 8.2, went wrong in 8.4.1 and was fixed in 8.4.2?
Simon
| -----Original Message-----
| From: Evan Laforge
On Wed, May 2, 2018 at 12:27 PM, Simon Peyton Jones
Wow. Could you open a ticket?
Done: https://ghc.haskell.org/trac/ghc/ticket/15114
I just tried with 8.2.2 which is what I have on this laptop, but it printed "all is well". Does that mean it was fine in 8.2, went wrong in 8.4.1 and was fixed in 8.4.2?
It seems likely!
participants (3)
-
Ben Gamari -
Evan Laforge -
Simon Peyton Jones