Clash not inlining certain floating point subtractions

[acairncross]

Compiling this:

{-# LANGUAGE DeriveGeneric #-}

import Clash.Prelude

topEntity :: MyFloat
topEntity = -1.0

data MyFloat = MyFloat {
  negative :: Bool,
  exponent :: Unsigned 8,
  mantissa :: UFixed 0 23
} deriving (Eq, Generic, BitPack)

instance Num MyFloat where
  (-) a b = bitCoerce @Float (bitCoerce a - bitCoerce b)

instance Fractional MyFloat where
  fromRational = bitCoerce @Float . fromRational

errors with:

<no location info>: error:
    Clash error call:
    Clash.Normalize.Transformations(857): Report as bug: caseCon error: case 1.0 of
      GHC.Types.F#[3891110078048108559] (y[6989586621679217777] :: GHC.Prim.Float#[3674937295934324758]) ->
        GHC.Types.F#[3891110078048108559] (GHC.Prim.minusFloat# x[6989586621679217773][LocalId] y[6989586621679217777][LocalId])
    CallStack (from HasCallStack):
      error, called at src/Clash/Normalize/Transformations.hs:857:3 in clash-lib-1.4.7-A3e0Bg7blinAlB0TODW2ZW:Clash.Normalize.Transformations

Clash seems to have trouble with the Float subtraction in (-) a b = bitCoerce @Float (bitCoerce a - bitCoerce b) needed to evaluate the literal -1 :: MyFloat.

Changing topEntity to -1 :: Float does compile on the other hand.

[christiaanb]

Ah, the issue is simply that we haven't added support for matching on Float literals, like we do for Integer and Natural literals:

clash-compiler/clash-lib/src/Clash/Normalize/Transformations.hs

Lines 761 to 853 in fa01fd9

	matchLiteralContructor
	:: Term
	-> Literal
	-> [(Pat,Term)]
	-> NormalizeSession Term
	matchLiteralContructor c (IntegerLiteral l) alts = go (reverse alts)
	where
	go [(DefaultPat,e)] = changed e
	go ((DataPat dc [] xs,e):alts')
	| dcTag dc == 1
	, l >= ((-2)^(63::Int)) && l < 2^(63::Int)
	= let fvs = Lens.foldMapOf freeLocalIds unitVarSet e
	(binds,_) = List.partition ((`elemVarSet` fvs) . fst)
	$ List.zipEqual xs [Literal (IntLiteral l)]
	e' = case binds of
	[] -> e
	_ -> Letrec binds e
	in changed e'
	| dcTag dc == 2
	, l >= 2^(63::Int)
	#if MIN_VERSION_base(4,15,0)
	= let !(IP ba) = l
	#else
	= let !(Jp# !(BN# ba)) = l
	#endif
	ba' = BA.ByteArray ba
	fvs = Lens.foldMapOf freeLocalIds unitVarSet e
	(binds,_) = List.partition ((`elemVarSet` fvs) . fst)
	$ List.zipEqual xs [Literal (ByteArrayLiteral ba')]
	e' = case binds of
	[] -> e
	_ -> Letrec binds e
	in changed e'
	| dcTag dc == 3
	, l < ((-2)^(63::Int))
	#if MIN_VERSION_base(4,15,0)
	= let !(IN ba) = l
	#else
	= let !(Jn# !(BN# ba)) = l
	#endif
	ba' = BA.ByteArray ba
	fvs = Lens.foldMapOf freeLocalIds unitVarSet e
	(binds,_) = List.partition ((`elemVarSet` fvs) . fst)
	$ List.zipEqual xs [Literal (ByteArrayLiteral ba')]
	e' = case binds of
	[] -> e
	_ -> Letrec binds e
	in changed e'
	| otherwise
	= go alts'
	go ((LitPat l', e):alts')
	| IntegerLiteral l == l'
	= changed e
	| otherwise
	= go alts'
	go _ = error $ $(curLoc) ++ "Report as bug: caseCon error: " ++ showPpr c
	matchLiteralContructor c (NaturalLiteral l) alts = go (reverse alts)
	where
	go [(DefaultPat,e)] = changed e
	go ((DataPat dc [] xs,e):alts')
	| dcTag dc == 1
	, l >= 0 && l < 2^(64::Int)
	= let fvs = Lens.foldMapOf freeLocalIds unitVarSet e
	(binds,_) = List.partition ((`elemVarSet` fvs) . fst)
	$ List.zipEqual xs [Literal (WordLiteral l)]
	e' = case binds of
	[] -> e
	_ -> Letrec binds e
	in changed e'
	| dcTag dc == 2
	, l >= 2^(64::Int)
	#if MIN_VERSION_base(4,15,0)
	= let !(IP ba) = l
	#else
	= let !(Jp# !(BN# ba)) = l
	#endif
	ba' = BA.ByteArray ba
	fvs = Lens.foldMapOf freeLocalIds unitVarSet e
	(binds,_) = List.partition ((`elemVarSet` fvs) . fst)
	$ List.zipEqual xs [Literal (ByteArrayLiteral ba')]
	e' = case binds of
	[] -> e
	_ -> Letrec binds e
	in changed e'
	| otherwise
	= go alts'
	go ((LitPat l', e):alts')
	| NaturalLiteral l == l'
	= changed e
	| otherwise
	= go alts'
	go _ = error $ $(curLoc) ++ "Report as bug: caseCon error: " ++ showPpr c

[alex-mckenna]

So a few things here:

• when you just use -1.0 apparently you end up with 0 - 1.0, i.e. fromInteger 0 - fromRational 1.0. In the debug log I was seeing the first argument to the topEntity replaced with EmptyCase until I implemented fromInteger too (GHC 9.0.2). This was needed to make the final version compile, so it's worth saying that you should at least always implement fromInteger for numbers so things don't mysteriously go wrong

• I disagree with @christiaanb's assessment of the fix. We match on Integer and Natural constructors because they have them, but FloatLiteral is Float# which has no constructors. The real culprit here was unpackFloat# and unpackDouble# which return boxed Float and Double but the primitive evaluation rule returned Float# and Double#. This means you end up with something like

  case 0.0# of
    F# x -> ...

  which obviously won't caseCon, but with the primitives fixed we end up with

  case F# 0.0# of
    F# x -> ...

  which does caseCon and normalization can finish.

However, this still doesn't make this particular example compile. We're missing the primitives F# and D# in the evaluator. With those added the example compiles and runs with no problems 🎉

PR incoming...