Skip to content

Add Clash.Class.Convert #2915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ADD: `Clash.Class.Convert`: Utilities for safely converting between various Clash number types
5 changes: 5 additions & 0 deletions clash-prelude/clash-prelude.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ Library
Clash.Class.Counter
Clash.Class.Counter.Internal
Clash.Class.Counter.TH
Clash.Class.Convert
Clash.Class.Convert.Internal.Convert
Clash.Class.Convert.Internal.MaybeConvert
Clash.Class.Exp
Clash.Class.HasDomain
Clash.Class.HasDomain.HasSingleDomain
Expand Down Expand Up @@ -428,10 +431,12 @@ test-suite unittests
Clash.Tests.BlockRam.Blob
Clash.Tests.Clocks
Clash.Tests.Counter
Clash.Tests.Convert
Clash.Tests.DerivingDataRepr
Clash.Tests.DerivingDataReprTypes
Clash.Tests.Fixed
Clash.Tests.FixedExhaustive
Clash.Tests.MaybeConvert
Clash.Tests.MaybeX
Clash.Tests.NFDataX
Clash.Tests.NumNewtypes
Expand Down
31 changes: 31 additions & 0 deletions clash-prelude/src/Clash/Class/Convert.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE MonoLocalBinds #-}
{-# LANGUAGE UndecidableInstances #-}

{- | Utilities for converting between Clash number types in a safe way. Its
existence is motivated by the observation that Clash users often need to convert
between different number types (e.g., 'Clash.Sized.Unsigned.Unsigned' to
'Clash.Sized.Signed.Signed') and that it is not always clear how to do so
properly. Two classes are exported:

* 'Convert': for conversions that, based on types, are guaranteed to succeed.
* 'MaybeConvert': for conversions that may fail for some values.

As opposed to 'Prelude.fromIntegral', all conversions are translatable to
synthesizable HDL.

== __Relation to @convertible@__
@clash-convertible@ is similar to the @convertible@ package in that it aims to
facilitate conversions between different number types. It has two key differences:
Comment on lines +18 to +19
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To change because it isn't a separate package


1. It offers no partial functions.
2. All its conversions are translatable to synthesizable HDL.

-}
module Clash.Class.Convert (
Convert (..),
MaybeConvert (..),
) where

import Clash.Class.Convert.Internal.Convert
import Clash.Class.Convert.Internal.MaybeConvert
164 changes: 164 additions & 0 deletions clash-prelude/src/Clash/Class/Convert/Internal/Convert.hs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you have instances covering BitVector, it might be also desirable to have instances converting from Bit (Bool). Something like a Bit to Word8 conversion sounds reasonable to me.

On the other hand, we should ask ourselves whether BitVector should be considered a number type? I'm still struggling with conceptual separation here, as if BitVector is a number type, what distinguishes it from Unsigned then in the end?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit/Bool: yes good point.

Num: I've got two answers:

  • For this PR specifically: IMO this class shouldn't take a stance on that -- anything that has a Num is a number type for its sake.
  • Zooming out a bit I feel that BitVector should be an "unstructured collection of bits". (In practice it probably isn't truly unstructured, but for the domain that it is being handled in it is.) From a theoretical POV this makes the Num instance for BitVector icky IMO. However, I also feel that the convenience of being able to use literals (e.g., 0xFFAA) to construct them and do basic arithmetic on them trumps my theoretical concerns (not to mention what a massive API changes it would be to remove the instance). In practice I just end up using it to signal to developers that bits are unstructured.

Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE GADTs #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE UndecidableInstances #-}

{-# OPTIONS_HADDOCK hide #-}

{-# OPTIONS_GHC -fplugin=GHC.TypeLits.KnownNat.Solver #-}

module Clash.Class.Convert.Internal.Convert where

import Prelude

import Clash.Class.BitPack
import Clash.Class.Resize
import Clash.Sized.BitVector
import Clash.Sized.Index
import Clash.Sized.Signed
import Clash.Sized.Unsigned

import GHC.TypeLits.Extra (CLog)
import GHC.TypeLits (KnownNat, type (<=), type (+), type (^))

import Data.Int (Int16, Int32, Int64, Int8)
import Data.Word (Word16, Word32, Word64, Word8)

{- $setup
>>> import Clash.Prelude
>>> import Clash.Class.Convert
-}

{- | Conversions that are, based on their types, guaranteed to succeed.

== __Laws__
A conversion is safe and total if a round trip conversion is guaranteed to be
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"safe and total" is not terminology introduced earlier

lossless. I.e.,

> Just x == maybeConvert (convert @a @b x)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason for mixing maybeConvert and convert in a single law? Wouldn't it make more sense to have a separate law per class, as a type not necessarily needs to be an instance of both?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, well it isn't imposed by anything, but it seems natural to me that everything that defines a Convert also defines a MaybeConvert. I get that that makes laws like these a bit iffy -- I'm happy to add a superclass constraint if that makes more sense to you. I'm not sure how to rephrase this law properly to just talk about convert.


for all values @x@ of type @a@. It should also preserve the numerical value
interpretation of the bits. For types that have an "Integral" instance, this
intuition is captured by:

> toInteger x == toInteger (convert @a @b x)

Instances should make sure their constraints are as \"tight\" as possible. I.e.,
if an instance exist, but the constraints cannot be satisfied, then
'Clash.Class.Convert.convertMaybe' should return 'Nothing' for one or more values in
the domain of the source type @a@:

> L.any isNothing (L.map (maybeConvert @a @b) [minBound ..])

Additionally, any implementation should be translatable to synthesizable RTL.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mention synthesizable HDL in Clash.Class.Convert. I'd suggest to keep the wording consistent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

-}
class Convert a b where
{- | Convert a supplied value of type @a@ to a value of type @b@. The conversion
is guaranteed to succeed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The classical wording in legal specification documents usually is "must be guaranteed to succeed", as it is a user requirement, in case you like to stick with that.


>>> convert (3 :: Index 8) :: Unsigned 8
3

The following will fail with a type error, as we cannot prove that all values
of @Index 8@ can be represented by an @Unsigned 2@:

>>> convert (3 :: Index 8) :: Unsigned 2
...

For the time being, if the input is an @XException@, then the output is too. This
property might be relaxed in the future.
-}
convert :: a -> b

instance (KnownNat n, KnownNat m, n <= m) => Convert (Index n) (Index m) where
convert = resize

instance (KnownNat n, KnownNat m, 1 <= n, n <= 2 ^ m) => Convert (Index n) (Unsigned m) where
convert !a = resize $ bitCoerce a

{- | Note: Conversion from @Index 1@ to @Signed 0@ is total, but not within the
constraints of the instance.
-}
instance (KnownNat n, KnownNat m, 1 <= n, CLog 2 n + 1 <= m) => Convert (Index n) (Signed m) where
convert !a = convert $ bitCoerce @_ @(Unsigned (CLog 2 n)) a

instance (KnownNat n, KnownNat m, 1 <= n, n <= 2 ^ m) => Convert (Index n) (BitVector m) where
convert !a = resize $ pack a

instance (KnownNat n, KnownNat m, 1 <= m, 2 ^ n <= m) => Convert (Unsigned n) (Index m) where
convert !a = bitCoerce $ resize a

instance (KnownNat n, KnownNat m, n <= m) => Convert (Unsigned n) (Unsigned m) where
convert = resize

{- | Note: Conversion from @Unsigned 0@ to @Signed 0@ is total, but not within the
constraints of the instance.
-}
instance (KnownNat n, KnownNat m, n + 1 <= m) => Convert (Unsigned n) (Signed m) where
convert = bitCoerce . resize

instance (KnownNat n, KnownNat m, n <= m) => Convert (Unsigned n) (BitVector m) where
convert !a = resize $ pack a

instance (KnownNat n, KnownNat m, n <= m) => Convert (Signed n) (Signed m) where
convert !a = resize a

instance (KnownNat n, KnownNat m, 1 <= m, 2 ^ n <= m) => Convert (BitVector n) (Index m) where
convert = unpack . resize

instance (KnownNat n, KnownNat m, n <= m) => Convert (BitVector n) (Unsigned m) where
convert = unpack . resize

{- | Note: Conversion from @BitVector 0@ to @Signed 0@ is total, but not within the
constraints of the instance.
-}
instance (KnownNat n, KnownNat m, n + 1 <= m) => Convert (BitVector n) (Signed m) where
convert = unpack . resize

instance (KnownNat n, KnownNat m, n <= m) => Convert (BitVector n) (BitVector m) where
convert = resize

instance (Convert (Unsigned 64) a) => Convert Word a where
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RHS parenthesis are redundant here.

Just noting this, as hlint won't complain about that, in case you prefer them being removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.. this is something fourmolu does by default

convert = convert . bitCoerce @_ @(Unsigned 64)
instance (Convert (Unsigned 64) a) => Convert Word64 a where
convert = convert . bitCoerce @_ @(Unsigned 64)
instance (Convert (Unsigned 32) a) => Convert Word32 a where
convert = convert . bitCoerce @_ @(Unsigned 32)
instance (Convert (Unsigned 16) a) => Convert Word16 a where
convert = convert . bitCoerce @_ @(Unsigned 16)
instance (Convert (Unsigned 8) a) => Convert Word8 a where
convert = convert . bitCoerce @_ @(Unsigned 8)

instance (Convert (Signed 64) a) => Convert Int a where
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would either not add any instances for Word and Int, as their sizes are machine dependent, or the source of the respective conversions should be restricted to at most 32 bit, as this is what base fixes independently from the underlying machine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh woops. I think this just fails to compile on any platform that is not 32-bit, as the bitCoerce would type error. This did cross my mind, but I never followed up on it -- I'll add some CPP foo.

convert = convert . bitCoerce @_ @(Signed 64)
instance (Convert (Signed 64) a) => Convert Int64 a where
convert = convert . bitCoerce @_ @(Signed 64)
instance (Convert (Signed 32) a) => Convert Int32 a where
convert = convert . bitCoerce @_ @(Signed 32)
instance (Convert (Signed 16) a) => Convert Int16 a where
convert = convert . bitCoerce @_ @(Signed 16)
instance (Convert (Signed 8) a) => Convert Int8 a where
convert = convert . bitCoerce @_ @(Signed 8)

instance (Convert a (Unsigned 64)) => Convert a Word where
convert = bitCoerce @(Unsigned 64) . convert
instance (Convert a (Unsigned 64)) => Convert a Word64 where
convert = bitCoerce @(Unsigned 64) . convert
instance (Convert a (Unsigned 32)) => Convert a Word32 where
convert = bitCoerce @(Unsigned 32) . convert
instance (Convert a (Unsigned 16)) => Convert a Word16 where
convert = bitCoerce @(Unsigned 16) . convert
instance (Convert a (Unsigned 8)) => Convert a Word8 where
convert = bitCoerce @(Unsigned 8) . convert

instance (Convert a (Signed 64)) => Convert a Int where
convert = bitCoerce @(Signed 64) . convert
instance (Convert a (Signed 64)) => Convert a Int64 where
convert = bitCoerce @(Signed 64) . convert
instance (Convert a (Signed 32)) => Convert a Int32 where
convert = bitCoerce @(Signed 32) . convert
instance (Convert a (Signed 16)) => Convert a Int16 where
convert = bitCoerce @(Signed 16) . convert
instance (Convert a (Signed 8)) => Convert a Int8 where
convert = bitCoerce @(Signed 8) . convert
160 changes: 160 additions & 0 deletions clash-prelude/src/Clash/Class/Convert/Internal/MaybeConvert.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE GADTs #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE UndecidableInstances #-}

{-# OPTIONS_HADDOCK hide #-}

{-# OPTIONS_GHC -fplugin=GHC.TypeLits.Extra.Solver #-}
{-# OPTIONS_GHC -fplugin=GHC.TypeLits.Normalise #-}
{-# OPTIONS_GHC -fplugin=GHC.TypeLits.KnownNat.Solver #-}

module Clash.Class.Convert.Internal.MaybeConvert where

import Clash.Class.BitPack
import Clash.Class.Resize
import Clash.Sized.BitVector
import Clash.Sized.Index
import Clash.Sized.Signed
import Clash.Sized.Unsigned

import GHC.TypeLits.Extra (CLog)
import GHC.TypeLits (KnownNat, type (<=), type (+), type (^))

import Data.Int (Int16, Int32, Int64, Int8)
import Data.Word (Word16, Word32, Word64, Word8)

{- $setup
>>> import Clash.Prelude
>>> import Clash.Class.Convert
-}

{- | Conversions that may fail for some values.

== __Laws__
A conversion is safe if a round trip conversion does not produce errors (also
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the word "safe" here.

see "Clash.XException"). I.e.,

> x == fromMaybe x (maybeConvert @a @b x >>= maybeConvert @b @a)

for all values @x@ of type @a@. It should also preserve the numerical value
interpretation of the bits. For types that have an "Integral" instance, this
intuition is captured by:

> toInteger x == fromMaybe (toInteger x) (toInteger (convert @a @b x))

If a conversion succeeds one way, it should also succeed the other way. I.e.,

> isJust (maybeConvert @a @b x) `implies` isJust (maybeConvert @a @b x >>= maybeConvert @b @a)

A conversion should succeed if and only if the value is representable in the
target type. For types that have a "Bounded" and "Integral" instance, this
intuition is captured by:

> isJust (maybeConvert @a @b x) == (i x >= i (minBound @b) && i x <= i (maxBound @b))

where @i = toInteger@.

Additionally, any implementation should be translatable to synthesizable RTL.
-}
class MaybeConvert a b where
{- | Convert a supplied value of type @a@ to a value of type @b@. If the value
cannot be represented in the target type, 'Nothing' is returned.

>>> maybeConvert (1 :: Index 8) :: Maybe (Unsigned 2)
Just 1
>>> maybeConvert (7 :: Index 8) :: Maybe (Unsigned 2)
Nothing

For the time being, if the input is an @XException@, then the output is too.
This property might be relaxed in the future.
-}
maybeConvert :: a -> Maybe b
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer the convention to call this function convertMaybe (similar to readMaybe in base), as the name then denotes the order of operation: "first convert and then put the potential result into the Maybe container". maybeConvert suggests to me that the function converts from a Maybe as the input.

Another advantage of convertMaybe is that TAB completion in the REPL will display both: convert and convertMaybe, as soon as you started typing c o n ... .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with either as far as naming schemes go. I do think we've already set a precedent in clash-prelude, i.e.:

martijn@volthe:~/code/clash-compiler$ egrep -r '^maybe[A-Z].*::'  clash-prelude/src | wc -l
7
martijn@volthe:~/code/clash-compiler$ egrep -r '^.*Maybe ::'  clash-prelude/src | wc -l
1

so I think we should continue that. (Or change that, but that's for another PR.)


instance (KnownNat n, KnownNat m) => MaybeConvert (Index n) (Index m) where
maybeConvert !a = maybeResize a

instance (KnownNat n, KnownNat m, 1 <= n) => MaybeConvert (Index n) (Unsigned m) where
maybeConvert !a = maybeResize $ bitCoerce @_ @(Unsigned (CLog 2 n)) a

instance (KnownNat n, KnownNat m, 1 <= n) => MaybeConvert (Index n) (Signed m) where
maybeConvert !a = maybeConvert $ bitCoerce @_ @(Unsigned (CLog 2 n)) a

instance (KnownNat n, KnownNat m, 1 <= n) => MaybeConvert (Index n) (BitVector m) where
maybeConvert !a = maybeResize $ pack a

instance (KnownNat n, KnownNat m) => MaybeConvert (Unsigned n) (Index m) where
maybeConvert !a = maybeResize $ bitCoerce @_ @(Index (2 ^ n)) a

instance (KnownNat n, KnownNat m) => MaybeConvert (Unsigned n) (Unsigned m) where
maybeConvert !a = maybeResize a

instance (KnownNat n, KnownNat m) => MaybeConvert (Unsigned n) (Signed m) where
maybeConvert !a = maybeResize $ bitCoerce @(Unsigned (n + 1)) $ extend a

instance (KnownNat n, KnownNat m) => MaybeConvert (Unsigned n) (BitVector m) where
maybeConvert !a = maybeResize $ pack a

instance (KnownNat n, KnownNat m) => MaybeConvert (Signed n) (Index m) where
maybeConvert n
| n < 0 = Nothing
| otherwise = maybeResize (bitCoerce @_ @(Index (2 ^ (n + 1))) (extend n))

instance (KnownNat n, KnownNat m) => MaybeConvert (Signed n) (Unsigned m) where
maybeConvert n
| n < 0 = Nothing
| otherwise = maybeResize (bitCoerce @(Signed (n + 1)) (extend n))

instance (KnownNat n, KnownNat m) => MaybeConvert (Signed n) (Signed m) where
maybeConvert !a = maybeResize a

instance (KnownNat n, KnownNat m) => MaybeConvert (Signed n) (BitVector m) where
maybeConvert n
| n < 0 = Nothing
| otherwise = maybeResize (pack @(Signed (n + 1)) (extend n))

instance (KnownNat n, KnownNat m) => MaybeConvert (BitVector n) (Index m) where
maybeConvert !a = maybeResize $ unpack @(Index (2 ^ n)) a

instance (KnownNat n, KnownNat m) => MaybeConvert (BitVector n) (Unsigned m) where
maybeConvert !a = maybeResize $ unpack @(Unsigned n) a

instance (KnownNat n, KnownNat m) => MaybeConvert (BitVector n) (Signed m) where
maybeConvert !a = maybeResize $ unpack @(Signed (n + 1)) $ extend a

instance (KnownNat n, KnownNat m) => MaybeConvert (BitVector n) (BitVector m) where
maybeConvert !a = maybeResize a

instance (MaybeConvert (Unsigned 64) a) => MaybeConvert Word a where
maybeConvert = maybeConvert . bitCoerce @_ @(Unsigned 64)
instance (MaybeConvert (Unsigned 64) a) => MaybeConvert Word64 a where
maybeConvert = maybeConvert . bitCoerce @_ @(Unsigned 64)
instance (MaybeConvert (Unsigned 32) a) => MaybeConvert Word32 a where
maybeConvert = maybeConvert . bitCoerce @_ @(Unsigned 32)
instance (MaybeConvert (Unsigned 16) a) => MaybeConvert Word16 a where
maybeConvert = maybeConvert . bitCoerce @_ @(Unsigned 16)
instance (MaybeConvert (Unsigned 8) a) => MaybeConvert Word8 a where
maybeConvert = maybeConvert . bitCoerce @_ @(Unsigned 8)

instance (MaybeConvert (Signed 64) a) => MaybeConvert Int a where
maybeConvert = maybeConvert . bitCoerce @_ @(Signed 64)
instance (MaybeConvert (Signed 64) a) => MaybeConvert Int64 a where
maybeConvert = maybeConvert . bitCoerce @_ @(Signed 64)
instance (MaybeConvert (Signed 32) a) => MaybeConvert Int32 a where
maybeConvert = maybeConvert . bitCoerce @_ @(Signed 32)
instance (MaybeConvert (Signed 16) a) => MaybeConvert Int16 a where
maybeConvert = maybeConvert . bitCoerce @_ @(Signed 16)
instance (MaybeConvert (Signed 8) a) => MaybeConvert Int8 a where
maybeConvert = maybeConvert . bitCoerce @_ @(Signed 8)

instance (MaybeConvert a (Unsigned 64)) => MaybeConvert a Word where
maybeConvert = fmap (bitCoerce @(Unsigned 64)) . maybeConvert
instance (MaybeConvert a (Unsigned 64)) => MaybeConvert a Word64 where
maybeConvert = fmap (bitCoerce @(Unsigned 64)) . maybeConvert
instance (MaybeConvert a (Unsigned 32)) => MaybeConvert a Word32 where
maybeConvert = fmap (bitCoerce @(Unsigned 32)) . maybeConvert
instance (MaybeConvert a (Unsigned 16)) => MaybeConvert a Word16 where
maybeConvert = fmap (bitCoerce @(Unsigned 16)) . maybeConvert
instance (MaybeConvert a (Unsigned 8)) => MaybeConvert a Word8 where
maybeConvert = fmap (bitCoerce @(Unsigned 8)) . maybeConvert
Loading