
#16111: Inconsistent behavior of Data.Bits.shiftL with different optimization levels and -fllvm -------------------------------------+------------------------------------- Reporter: RyanGlScott | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Compiler | Version: 8.6.3 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: Incorrect result | Unknown/Multiple at runtime | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by harpocrates): * cc: harpocrates (added) Comment: `shiftL` is ''not'' supposed to be called with a negative number of bits (that's what `shift` is for). It's documented in the Haddock for `shiftL`:
Shift the argument left by the specified number of bits (which must be non-negative).
Without any heavy optimizations, `shiftL 1 hm` turns into `shiftL#` turns into the `uncheckedShiftL#` primop, which explicitly says that a negative number of bits produces undefined output. I suspect it is nonetheless usually deterministic, hence the `True` outputs. On `-O2`, we'll trigger the `uncheckedShiftL#` builtin rule. That rule takes one look at the negative shift bits and decides to "optimize" this into a runtime error that prints out `"Bad shift length"` and the shift length. The only case that isn't yet covered is the `-O1 -fllvm` case. It's likely that the LLVM call we produce for `uncheckedShiftL#` also has undefined behaviour for negative shift values, and LLVM applies some more optimizations ultimately resulting in the `False` output. ---- This looks like a bad API: we really should have `shiftL :: a -> Word -> a` instead of `shiftL :: a -> Int -> a` (and same for `shiftR`). Instead of stomaching that breakage, we could also just add an extra check to the `Word`, `Int`, etc. instances of `shiftL` to guard against negative inputs: {{{ instance Bits Word where {- snipped -} (W# x#) `shift` (I# i#) | isTrue# (i# >=# 0#) = W# (x# `shiftL#` i#) | otherwise = W# (x# `shiftRL#` negateInt# i#) (W# x#) `shiftL` (I# i#) | isTrue# (i# >=# 0#) = W# (x# `shiftL#` i#) | otherwise = error "shiftL: expected non-negative number of bits" }}} Or even: deprecate `shiftL`/`shiftR` entirely in favour of `shift` (and let the default definitions, which fall back to the safer `shift`, take effect). Thoughts? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/16111#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler