Skip to content
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

Add support for Haskell types: Word(N), Int(N), and Char #108

Merged
merged 31 commits into from
Jan 7, 2016

Conversation

christiaanb
Copy link
Member

Add support for:

  • Int8
  • Int16
  • Int32
  • Int64
  • Word
  • Word8
  • Word16
  • Word32
  • Word64
  • Char

Where 'N' is defined by the architecture on which the compiler
is going to run. Previously we always treated Int as a signed
32-bit number. With this change we will have a better match
between simulation and eventual circuit, as most computers
these days have 64-bit integers.
as long as literals fall within VHDLs integer range. Otherwise
they must be printed as bitstrings.
Needed when the integer value that needs to be converted is larger
than 2^31.
christiaanb added a commit that referenced this pull request Jan 7, 2016
Add support for Haskell types: Word(N), Int(N), and Char
@christiaanb christiaanb merged commit 3b4f9fe into master Jan 7, 2016
@Ericson2314
Copy link
Contributor

Do we support Natural? There are a bunch of changes in clash-prelude I wanted to make switching things from Integer/Int to Natural/Word.

@christiaanb
Copy link
Member Author

Natural is just as bad as Integer, it can only be improperly supported in CLaSH. I mean, I don't know what size Integer is, so I just make it 32 bits.

@Ericson2314
Copy link
Contributor

Oh, now that I think about it, the integer/natural use-cases I cared about we didn't synthesize anyways, (such as underlying type for BitVector). So this should be all that is needed.

@Ericson2314
Copy link
Contributor

On a different note, I am concerned about the final commit, that means two different developers on different arch could synthesize same code differently. Since clash is conceptually a cross-compiler, I am not sure inheriting host-arch stuff like that is a good idea.

At the very least, might be good to ensure the prelude doesn't use any word/int as an implementation detail that would trigger such behavior.

@Ericson2314
Copy link
Contributor

For example of both: https://github.com/clash-lang/clash-compiler/blob/master/clash-systemverilog/primitives/CLaSH.Prelude.RAM.json#L7-L8 ought to be unsigned, and definitely shouldn't be host-arch-dependent as a wrapper function hides the use of Int as an implementation detail.

@Ericson2314
Copy link
Contributor

@christiaanb
Copy link
Member Author

I will take a look at the primitives/prelude. Until now, I've just been kinda hoping that people don't use Vectors or blockRAMs of size 2^31-1. That's the only time things can go wrong: VHDL simulation and synthesis tools don't support vectors/blockrams that are larger, I think. The reason is that those vectors could not be indexed, as 2^31-1 is the largest integer value: in VHDL, you can only index with values of type integer.

@christiaanb
Copy link
Member Author

Also, the reason I made the translation of Int and Word architecture-dependent, is overflow behaviour: I want overflow behaviour to be the same as the Haskell simulation on the system on which we are synthesising.

@Ericson2314
Copy link
Contributor

Re post 1: that all makes sense, sounds like we should be using Word32 in both cases then?

@Ericson2314
Copy link
Contributor

Re post 2: Maybe the thing to do is just not support Int or Word at all, or give a warning? The first especially is rather dogmatic, but either host-arch-dependence or a simulation-synthesis mismatch are really infuriating bugs to fall victim to.

@Ericson2314
Copy link
Contributor

To elaborate, there was some talk with Rust on what the default integer type should be (rust-lang/rfcs#212 and maybe other places). One thing I learned is that 32-bit math is actually faster / just as fast on most x86-64 chips. IMO the only reason to use Word/Int is pointer arithmetic or speed. If Word32 is just as fast, VHDL effective has a "pointer-size" of 32-bits, and any larger blockrams/bitvectors/etc would be grossly inefficient to simulate in Haskell regardless, I see no valid reason to ever want Int or Word with CLaSH.

@christiaanb
Copy link
Member Author

Well, yes, I don't want to use Int (or Word) either, but Int and Integer are Haskell's "staple" data types. They show up everywhere. I need to support Int and Word, or otherwise I can't support most of the standard Haskell prelude. The reason index_int takes an Int argument, is because I want to be able to index into Vectors with any data type that is an instance of Enum. And Enum's fromEnum type is:

fromEnum :: Enum a => a -> Int

@Ericson2314
Copy link
Contributor

Well, yes, I don't want to use Int (or Word) either, but Int and Integer are Haskell's "staple" data types. They show up everywhere. I need to support Int and Word, or otherwise I can't support most of the standard Haskell prelude.

Then I would do the warning so users can audit when these show up. Somewhat relatedly, how does fromIntegral = fromInteger . toInteger work when the input and output types are >32 bits?

The reason index_int takes an Int argument, is because I want to be able to index into Vectors with any data type that is an instance of Enum. And Enum's fromEnum type is...

Does using Int mean we rely on the backends to treat negative indices the same way? Negative indices, overflow, VHDL's 32 bit restriction---seems to me like we have the perfect storm of things to surmount. This seems just more bullet-proof to me:

index_word32 :: KnownNat n => Vec n a -> Word32 -> a
index_word32 xs i@(W32# n0) = sub xs n0
  where
    sub :: Vec m a -> Word32# -> a
    sub Nil              _ = undefined
    sub (y `Cons` (!ys)) n = if isTrue# (n ==# 0#)
                             then y
                             else sub ys (n -# 1#)
{-# NOINLINE index_word32 #-}

(!!) :: (KnownNat n, Enum i) => Vec n a -> i -> a
xs !! i = if
   | i' < 0 = error "CLaSH.Sized.Vector.(!!): negative index"
   | i' > fromIntegral (maxBound :: Word32) :: Int = error "CLaSH.Sized.Vector.(!!): indices over 2^32-1 unsupported due to VHDL restrictions"
   | i' > maxIndex xs = error (P.concat [ "CLaSH.Sized.Vector.(!!): index "
                                        , show i
                                        , " is larger than maximum index "
                                        , show (maxIndex xs)
                                        ])
   | otherwise = index_word32 xs 
 where i' = fromEnum i
{-# INLINE (!!) #-}

@christiaanb
Copy link
Member Author

So for the above code, I have no choice but to give a warning then... because It uses Int, due to:

fromEnum :: Enum a => a -> Int

I'm not saying I'm happy with the current situation. But it's a compromise. And like I said, the only time the Int's in the primitives become a problem is when people start creating vectors long length 2^31!

@christiaanb
Copy link
Member Author

I will add a flag, with which you can set the number of bits that are used to encode 'Int' and 'Integer', where it defaults to the "native" width of the machine the compiler is build for. And I'll update the documentation to highlight this aspect of the translation of standard Haskell data types.

@Ericson2314
Copy link
Contributor

Good point about the would-be int-warning in my revised code. Heh, I had been proposing the warning and that change separately in my mind, and completely forgot about the interaction.

Furthermore, my experience just using Verilog and VHDL for synthesis led me to miss that for simulation purposes, Verilog and VHDL do something sane with out-of-bounds indexing, so there is no need for us to handle negative numbers instead of the output language.

Seeing all that, I think the flag you propose is the best we can do. Thanks!

christiaanb added a commit that referenced this pull request Jan 12, 2016
* The default is the system's default word with
* Integer is now treated as a Signed N-bit, instead of a HDL native integer

This is a follow-up to the discussion in #108
@christiaanb christiaanb deleted the haskellprims branch February 8, 2016 07:57
christiaanb added a commit that referenced this pull request Sep 6, 2018
Fix some broken links to the GHC manual
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants