mkRegFileFullLoad does not actually Load if index_t is 0 bits.

[rossc719g]

So.... I acknowledge that this is a bit of a silly thing to want to do...
But I do actually have reasons... I promise. :-)

I traced the issue down to the wrapRegFile function in RegFile.bs:

Specifically, this bit:

(Lines 90~98)

  else if valueOf si == 0 then
    module
      _a :: Reg a
      {-# hide #-}
      _a <- if isWCF then mkConfigRegU else mkRegU
      interface
        upd _ x = _a := x
        sub _ = _a
  else

When si (the size in bits of the index_t parameter) is 0, then it just creates a register, since RegFile.v can't handle the address size being 0 bits.

But, if the module being wrapped is mkRegFileFullLoad (or presumably mkRegFileLoad) then the "Load" part does not get implemented, and the file is never read.

The most obvious solution (if this is worth fixing at all), it to make some Load variants of some registers (at least mkRegLoad and mkConfigRegULoad), and use those in place of the mkConfigRegU and mkRegU on line 94.

I dunno if it is worth fixing, but it should be a pretty easy thing to do.
And who knows... Maybe "Load" variants of registers are also useful things for some people (like me). :-)

What do you think?

[quark17]

Interesting. Thanks for reporting!

A new Verilog primitive would be reasonable. I first wondered whether it should be RegFileLoad0.v, as there are other primitives for which we have separate 0 variants without the ports that would have to be zero-size (RWire0.v, FIFO10.v, etc). But I think you're right that a load-able register would be more generally useful, and maybe cleaner (being analogous to how the non-load RegFiles are implemented).

I then wondered if it shouldn't be RegUNLoad.v, to distinguish between load versions of the other register types (RegN, RegA), but then I realized that this is not an orthogonal property but another type. But actually, now I'm not sure. For RegFileLoad, the values are loaded in an initial block, and reset of the module doesn't change the values (doesn't cause a re-load). For RegFileLoad with zero-width index (one element), we could use a register that is loaded during initial and which is unaffected by reset. However, if I were inventing a load-able register, I could also imagine a register that is loaded on reset. I assume that you're proposing the former. Are you suggesting it be called mkRegULoad (and mkConfigRegULoad) with Verilog primitive RegUNLoad.v, or that it be called mkRegLoad (and mkConfigRegLoad) with Verilog primitive RegLoad.v?

[quark17]

Two other things: (1) A new primitive would need to be added for Bluesim as well. (2) If it's billed as a register, then it should probably be added to the register inlining stage. And, well, (3) tests for Verilog, Bluesim, and inlining would be needed. (It may also be worth testing it in BDW -- I don't think any special handling is needed there, but worth checking.)

Adding a new Verilog primitive is incredibly simple. Adding it for Bluesim and inlining would add a little more work, but should be straightforward.

[rossc719g]

To be honest, I am not exactly sure what I am suggesting specifically.
I had not gotten as far as considering what happens during reset.

It does feel intuitively like a Reg should load on reset.. but I think I would have said the same for a RegFile... but they don't, so my intuition might be off?

Maybe a RegFileLoad0.v is the best solution since it pulls on the fewest loose threads? :-)

[mieszko]

Does your use case involve the block that uses this register ever getting synthesized? (Assuming you can share that much, of course.)

Off the top of my head, I can think of three use cases:

• in testbenches,
• to emulate initialization with a scanchain at BS level (but then how do you scan stuff out?), and
• to initialize registers when a FPGA bitstream is generated (at least some FPGA tools do this w/ initial blocks).

For the first it probably makes no difference what one does, but for the second and third one might want to at least consider interaction with various synthesis and backend tools.

Also, if this ends up being implemented for “normal” registers, I'm not sure I'd want them to be inlined. If they're separate modules, I could swap them out for other reg modules without the initial block. (Ideally this wouldn't make any diff with judicious use of translate_off, but people often have various passes that parse the RTL and add/edit various things.)

[rossc719g]

Does your use case involve the block that uses this register ever getting synthesized?

TL;DR:
Nope. Its a testbench.

Longer answer:
I was implementing a mechanism for memories to be preinitialized for testing from programmatically created memory files. The initialization files are unique for each memory in the design (I.e., not just all 0s or anything). I use string parameters for each module that contains (directly or indirectly) a memory. I build up the string when instantiating the submodules. To help make it easier, I use utility functions to create rows, columns, or grids of modules which themselves might contain rows columns or grids, etc.. The issue came up when I needed a way to test the utility functions. I made a dummy module with "something that did a load", and I figured that a 1-element RegFile was all that I needed for the test. I spent a long time debugging, and thinking that I had just written bad code somewhere. After I ran out of places for a bug to hide, I went and read through RegFile.bs out of desperation, and saw where it was swapping out the RegFile for a non-loading reg. As soon as I switched to a 2-element RegFile, it all worked fine. (Ignoring all the warnings about the file not having enough data to fill the "memory").

[quark17]

What we could do for now is put an assertion in the mkRegFile*Load that produces an error if instantiated with index width zero. So then no one is surprised by it and wasting time debugging (sorry!).

I'd suggest changing wrapRegFile to not take isWCF as an argument -- which is being used to decide which register to instantiate in the zero-width index case -- and instead to take the module to instantiate for that case. This would make wrapRegFile take two modules as arguments. For Load variants, that can eventually be a load-able register, but for now can be a call to errorM.

[quark17]

Also, if this ends up being implemented for “normal” registers, I'm not sure I'd want them to be inlined. If they're separate modules, I could swap them out for other reg modules without the initial block. (Ideally this wouldn't make any diff with judicious use of translate_off, but people often have various passes that parse the RTL and add/edit various things.)

Note that the existing register modules already have initial blocks (as do most of the Verilog primitives) and they are guarded by `ifdef BSV_NO_INITIAL_BLOCKS, so they can be removed if it's a problem. There is also a flag for turning off inlining of RegN/RegUN/RegA, which is separate from the flags controlling inlining of wires and CRegs. We could have a separate flag for load-able registers, or just lump it in with the ordinary registers.

[rossc719g]

I wonder if just a warningM would probably be sufficient to save the debugging time.