Suggestion for prim_util.svh

[NilsGraf]

Would it make sense to combine prim_util.svh with prim_assert.sv into one file?

My suggestion would be:

1. Rename prim_assert.sv to prim_util.sv.
2. Move contents from prim_util.svh into prim_util.sv.

That way, all helper functions and macros are in a single file and don't require any include files. Note that prim_assert.sv is not an include file and therefore doesn't require an include line in each file.

[rswarbrick]

I don't think I understand the last comment. prim_assert.sv defines various macros. These definitely need to be included into files that use them. Maybe its filename should actually be prim_assert.svh: I'm not sure whether there's a good reason that it isn't.

Re combining the two: I don't really have strong feelings but I can't think of any benefit of doing so.

[imphil]

Thanks @NilsGraf for opening this issue, I wanted to get back to you on your email suggesting exactly that.

A couple points from my side:

• prim_assert.sv contains include file content, so it is an include file, and we need to keep it that way.
• It's named .sv instead of .svh for historic reasons, it would make sense to change that.
• prim_assert.sv currently also defines a package; this part is slightly ugly and shouldn't be in an include file. It currently works because of the include guards. I wouldn't mind cleaning that up.
• Finally, on the question of one vs two include files: No strong feelings either way, especially in this case where both files are kind of a "catch-all". In general, including more than one file doesn't seem odd to me.

[NilsGraf]

Ah, I see. Originally, no RTL file had an include prim_assert.sv line. prim_assert.sv was just at the beginning of each filelist, which I think worked well. I didn't realize that nowadays all RTL files have an 'include prim_assert.sv` line. I personally like the old way better because it removes the need for the include line in each file.

[imphil]

The "old way" was working only by chance, unfortunately. For a the full story see https://github.com/lowRISC/ibex/commit/23d4243acd4973c27e92ef82b86c9e721026449f.

[NilsGraf]

Thank you for the link, I wasn't aware of this. The "old mechanism" has been used successfully for many chips, but I guess they were using commercial EDA tools.

Would it make sense to modify Fusesoc/edalize to support this? Adding @olofk

I believe the "old way" had the following advantages:

1. Less noisy source code as it doesn't require an include line in each file.
2. Doesn't need ifdef guards to avoid defining things multiple times.
3. Perhaps compiles faster because each include line results in an additional file access (so file prim_assert.sv is read 100 times if you have 100 RTL files that include it).

[olofk]

@NilsGraf It's not so easy. Edalize support 20+ different tools and they all behave slighly different. Many tools, as you say, allow you to put include files first in a list (this is normally referred to as multi file compilation units) but not all. Some Edalize backends also have options to use multi or single file compilation units. Vivado for example requires you to declare these files specifically as global include files as can be seen with SweRV where we need to source this file for a similar situation.

Implicit include files are also hard to handle in some cases. Imagine having a uart.vh and a spi.vh that both sets a DATA_WIDTH define to different values. Which will be used where if we just implcitly include the both files. This might be ok for a single project where you're in control of all sources, but it will quickly break down when you add IP from different vendors or groups so in the light of that I'd say that explicitly including works better.

Ideally, FuseSoC/Edalize should handle implcit include files just as well as the explicit ones but it inevitably becomes a trickier problem when porting between tools.

[NilsGraf]

Thanks for your response. Actually, prim_assert.sv is not setting DATA_WIDTH parameters. It's defining a few global macros such as ASSERT_*. But I could see an issue if another IP defines the same macro names.

I'm curious to know which EDA tools don't support implicit include files. Thanks!

[olofk]

I'm curious to know which EDA tools don't support implicit include files. Thanks!

As mentioned above, Vivado needs these to be explicitly marked as global include files through its tcl interface. QuestaSim (and I think RivieraPro) needs to be setup for mfcu mode. For Quartus I think it's supported in the Pro version, but not in the Standard and Lite versions. Synplify Pro has a switch for Multi file compilation units. Not sure if that applies here though

[NilsGraf]

Thinking about this some more: If another IP defined the same global macro ASSERT then this would be always an issue, even with explicit includes. The lint tool (and simulator) will flag macro redefinition.

Note that prim_assert.sv and prim_util.svh macros are global macros, they are included before the module in each file (see here) (to be local macros they would need to be included inside the module itself, which would also require undef at the end of each file).

[NilsGraf]

As mentioned above, Vivado needs these to be explicitly marked as global include files through its tcl interface. QuestaSim (and I think RivieraPro) needs to be setup for mfcu mode. For Quartus I think it's supported in the Pro version, but not in the Standard and Lite versions. Synplify Pro has a switch for Multi file compilation units. Not sure if that applies here though

Could Fusesoc/edalize take care of this (e.g. set up QuestaSim for mfcu mode)? Thanks!

[olofk]

Yes. mfcu mode for questasim will be added as an option

[imphil]

prim_util.svh is now prim_util_pkg.sv for different reasons.