BlockRam write logic is clumsy to say the least

[ggreif]

(Block)Ram write logic involves 3 distinct signals which are semantically coupled. Why not also represent them together? I came up with this adapter that consolidates the write logic to a Maybe (addr, dta):

maybeWrite :: (Signal addr -> Signal addr -> Signal Bool -> Signal dt -> Signal dt)
           -> Signal (Maybe (addr, dt)) -> Signal addr -> Signal dt
maybeWrite ram wr rd = ram wrAddr rd wrEn wrData
  where apart (Just (addr, dt)) = (True, addr, dt)
        apart Nothing = (False, undefined, undefined)
        (wrEn, wrAddr, wrData) = unbundle (apart <$> wr)

This invites the whole zoo of utilities dealing with Maybe. Which is a good thing.

What do you think? Would a pull request be accepted for inclusion? Maybe even make this the default interface to (block)Rams?

[christiaanb]

The blockRam functions are indeed very simple wrappers around blockRam#. The primitive itself does not use Maybe, because sum-of-product types are not native to VHDL/Verilog.

Of course, the limitations of existing HDLs shouldn't restrict the CLaSH prelude :-) I favour changing the default interface of the blockRam functions (but not of the blockRam# primitive). However, given that this is a breaking change, I want to postpone it until GHC 8 is released, and all of CLaSH's dependencies also build on GHC 8. The reason is that there are going to be more breaking changes in the next release of the CLaSH prelude, using features only available in GHC 8; and I'd rather have many breaking changes in a single release than a some breaking changes spanning multiple releases.

[ggreif]

Cool thing, I can wait :-)

[christiaanb]

Yeah, me neither. Too bad GHC 8 is riddled with bugs introduced by both the type-in-kind and visible type applications patches. I think it'll be at least a month before GHC 8 can be released.

[polygonhell]

If your changing blockRAM's, is there any reason their isn't a true dual ported version? I know Xilinx supports this, and I thought Altera did as well. I ended up adding my own primitive and wrappers to support this.

[christiaanb]

Indeed, both Altera and Xilinx support true dual ported blockRam's. However, I seem to remember that they have different recommended HDL coding styles for true dual ported blockRam inference.

@polygonhell: are you working with Xilinx or Altera? if you're working with Xilinx could you give me the primitive you created? I'll test it on altera then.

[polygonhell]

I'm working with a couple of different Xilinx parts, I don't have an altera part to test, but I'm pretty sure I more or less copied the pattern from a Blog that claimed BlockRAM would be inferred on both. The primitive is here https://github.com/polygonhell/J1LikeCPU the files are DPBRam.json and DPBRam.hs.

[christiaanb]

@polygonhell I'm confused by your Haskell model and the corresponding VHDL. If I read your Haskell model, it seems that, when wreA is true, then doutA is equal to dinA, correct? However, when I look at your VHDL, when wreA is true, doutA retains its old value, i.e. not even the old value at address addrA, but the value of doutA from the previous cycle.

Am I missing something?

[polygonhell]

Your correct that's an error, in my case I actually don't care what dout is when wrE is true, but both models ought to at least agree. I can take a look once I get back from my current business trip.

[polygonhell]

I apologize for this taking as long as it has.

I think I've fixed the semantics issue in the blockRAM, the semantics are now write first, which is what the haskell was doing.

This is the synthesis report I get from ISE, I'll check it on Vivado later. FWIW I pretty much cut and pasted the VHDL from http://danstrother.com/2010/09/11/inferring-rams-in-fpgas/ (albeit incorrectly), he claims it's inferred on both Xilinx and Altera, and has equivalent Verilog code on the same page.

Synthesizing (advanced) Unit . INFO:Xst:3226 - The RAM will be implemented as a BLOCK RAM, absorbing the following register(s):

| ram_type           | Block                               |          |
-----------------------------------------------------------------------
| Port A                                                              |
|     aspect ratio   | 16384-word x 32-bit                 |          |
|     mode           | write-first                         |          |
|     clkA           | connected to signal <system1000>    | rise     |
|     weA            | connected to signal <eta_i2>        | high     |
|     addrA          | connected to signal <eta_i3>        |          |
|     diA            | connected to signal <eta_i4>        |          |
|     doA            | connected to signal <dpRamFileMain_dpRamFilezm_3_n_14.doutB> |          |
-----------------------------------------------------------------------
| optimization       | area                                |          |
-----------------------------------------------------------------------
| Port B                                                              |
|     aspect ratio   | 16384-word x 32-bit                 |          |
|     mode           | write-first                         |          |
|     clkB           | connected to signal <system1000>    | rise     |
|     addrB          | connected to signal <pTS_i1>        |          |
|     doB            | connected to signal <dpRamFileMain_dpRamFilezm_3_n_14.doutA> |          |
-----------------------------------------------------------------------
| optimization       | area                                |          |
-----------------------------------------------------------------------

[polygonhell]

It appears to be correctly inferred in Vivado, I'd need to build a more thorough test to be sure, unfortunately the reports it produces are a lot harder to decode, it's certainly adding BlockRAM to the design and it's generating a Bit file, and it appears to have correctly inferred write first. I don't have any Altera parts here to validate.

[christiaanb]

blockRam uses a Maybe (addr, a) write port as of clash-prelude-0.11.