search

bespoke-silicon-group / basejump_stl

BaseJump STL: A Standard Template Library for SystemVerilog
http://bjump.org/
Other
470 stars 96 forks source link

RFC: Wrap $display statements in a DEBUG macro? #115

Open drichmond opened 4 years ago

drichmond commented 4 years ago

(x-post to BSG Manycoree) It seems unnecessary to have $display statements enabled for every simulation run. Printing information on every run can slow the simulator down, or clog with unnecessary information.

In BSG F1 we have a printing header. bsg_pr_debug statements are only enabled when the DEBUG macro is defined. This can be defined globally, or on a per-file basis.

Instead of putting raw $display statements in BaseJump Code, it would be cool if we could wrap $display in a macro that is only enabled when DEBUG is defined. Like in C/C++ we could enable this on a per-file basis (by putting define DEBUG include "bsg_basejump_printing.vh") or globally when compiling the simulation executable

dpetrisko commented 4 years ago

Agree with the high level idea. Global defines are pretty finicky with compile order and weird tool inconsistencies, so I would suggest using parameters for this. This would allow us to enable on a per-module, per-file or global basis depending on how the parameters are set.

Here's an example of how to use parameters to enable debug to null, display, or file

https://github.com/black-parrot/pre-alpha-release/blob/master/bp_common/src/include/bp_common_defines.vh

drichmond commented 4 years ago

I agree with your assessment - global defines (and defines in general) can be finicky.

Is it possible to get the best of both worlds? Could we use bind, or a package, to set which modules to debug at a global scope?

taylor-bsg commented 4 years ago

Generally we have a debug_p parameter that controls this information. Some modules have different debug levels.

What kind of display statements are you referring to?

Generally we are friendly to one-time statements that print at the beginning, in particular for things that change depending on the simulation you are running. Things that fire and fire over and over we do not like to have turned on long term.

Legitimate errors and warnings we generally want on all the time but can add some code to silence during reset.

M

On Thu, Jul 25, 2019 at 8:42 AM Dustin Richmond notifications@github.com wrote:

(x-post to BSG Manycoree) It seems unnecessary to have $display statements enabled for every simulation run. Printing information on every run can slow the simulator down, or clog with unnecessary information.

In BSG F1 we have a printing header. bsg_pr_debug statements are only enabled when the DEBUG macro is defined. This can be defined globally, or on a per-file basis.

Instead of putting raw $display statements in BaseJump Code, it would be cool if we could wrap $display in a macro that is only enabled when DEBUG is defined. Like in C/C++ we could enable this on a per-file basis (by putting define DEBUG above include "bsg_basejump_printing.vh") or globally when compiling the simulation executable

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=AEFG5AB2JFVVFZ232YOXD3DQBHCXPA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HBQPFTA, or mute the thread https://github.com/notifications/unsubscribe-auth/AEFG5AD52NVA5Q6KVGJ7NTLQBHCXPANCNFSM4IG37MJQ .

drichmond commented 4 years ago

I'm referring to this in the Manycore (and a few other examples). These are useful -- for example Borna and I used them to debug -- but they're not clogging the simulation output for CUDA programs. My concern is that these statements are actually slowing down large scale simulation like they would in a C/C++ program.

https://github.com/bespoke-silicon-group/bsg_manycore/blob/64858dd07eaa4b7f8a767b89edc073947bb60bd6/v/vanilla_bean/network_rx.v#L330

taylor-bsg commented 4 years ago

I see — you see these every time a CUDA kernel is launched?

On Thu, Jul 25, 2019 at 10:05 AM Dustin Richmond notifications@github.com wrote:

I'm referring to this in the Manycore (and a few other examples). These are useful -- for example Borna and I used them to debug -- but they're not clogging the simulation output for CUDA programs. My concern is that these statements are actually slowing down large scale simulation like they would in a C/C++ program.

https://github.com/bespoke-silicon-group/bsg_manycore/blob/64858dd07eaa4b7f8a767b89edc073947bb60bd6/v/vanilla_bean/network_rx.v#L330

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=AEFG5AGBRTNCIMDUN76PYMLQBHMOBA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD22DOBQ#issuecomment-515127046, or mute the thread https://github.com/notifications/unsubscribe-auth/AEFG5ADXNYZOCTG533XEHHTQBHMOBANCNFSM4IG37MJQ .

drichmond commented 4 years ago

CUDA, SPMD, etc. CUDA is just the worst, since it freezes/unfreezes tiles repeatedly

On Jul 26, 2019, at 1:46 PM, taylor-bsg notifications@github.com wrote:

I see — you see these every time a CUDA kernel is launched?

On Thu, Jul 25, 2019 at 10:05 AM Dustin Richmond notifications@github.com wrote:

I'm referring to this in the Manycore (and a few other examples). These are useful -- for example Borna and I used them to debug -- but they're not clogging the simulation output for CUDA programs. My concern is that these statements are actually slowing down large scale simulation like they would in a C/C++ program.

https://github.com/bespoke-silicon-group/bsg_manycore/blob/64858dd07eaa4b7f8a767b89edc073947bb60bd6/v/vanilla_bean/network_rx.v#L330

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=AEFG5AGBRTNCIMDUN76PYMLQBHMOBA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD22DOBQ#issuecomment-515127046, or mute the thread https://github.com/notifications/unsubscribe-auth/AEFG5ADXNYZOCTG533XEHHTQBHMOBANCNFSM4IG37MJQ .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=ABQVI5GB4SBR2TPHQ4INUGLQBNPCVA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD25VQMI#issuecomment-515594289, or mute the thread https://github.com/notifications/unsubscribe-auth/ABQVI5BQSXUOWQ5IF37XPH3QBNPCVANCNFSM4IG37MJQ.

taylor-bsg commented 4 years ago

I see — this used to be something that only happens once so it was not disruptive, and it was helpful to know when a tile was supposed to start running. Solution might be to replace with val watcher from BaseJump STL bsg_test, which allow you to say “trigger only once”.

M

On Fri, Jul 26, 2019 at 1:47 PM Dustin Richmond notifications@github.com wrote:

CUDA, SPMD, etc. CUDA is just the worst, since it freezes/unfreezes tiles repeatedly

On Jul 26, 2019, at 1:46 PM, taylor-bsg notifications@github.com wrote:

I see — you see these every time a CUDA kernel is launched?

On Thu, Jul 25, 2019 at 10:05 AM Dustin Richmond < notifications@github.com> wrote:

I'm referring to this in the Manycore (and a few other examples). These are useful -- for example Borna and I used them to debug -- but they're not clogging the simulation output for CUDA programs. My concern is that these statements are actually slowing down large scale simulation like they would in a C/C++ program.

https://github.com/bespoke-silicon-group/bsg_manycore/blob/64858dd07eaa4b7f8a767b89edc073947bb60bd6/v/vanilla_bean/network_rx.v#L330

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub < https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=AEFG5AGBRTNCIMDUN76PYMLQBHMOBA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD22DOBQ#issuecomment-515127046 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AEFG5ADXNYZOCTG533XEHHTQBHMOBANCNFSM4IG37MJQ

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=ABQVI5GB4SBR2TPHQ4INUGLQBNPCVA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD25VQMI#issuecomment-515594289>, or mute the thread < https://github.com/notifications/unsubscribe-auth/ABQVI5BQSXUOWQ5IF37XPH3QBNPCVANCNFSM4IG37MJQ .

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=AEFG5AHJXXCMUPQSFYUFIDTQBNPGXA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD25VSQI#issuecomment-515594561, or mute the thread https://github.com/notifications/unsubscribe-auth/AEFG5ADLXHG6UPPMGR6PISLQBNPGXANCNFSM4IG37MJQ .

drichmond commented 4 years ago

That would be an acceptable solution - But there’s a good argument to have it display repeatedly too - that’s how Borna and I found a bug in our code during the tapeout

On Jul 26, 2019, at 2:06 PM, taylor-bsg notifications@github.com wrote:

I see — this used to be something that only happens once so it was not disruptive, and it was helpful to know when a tile was supposed to start running. Solution might be to replace with val watcher from BaseJump STL bsg_test, which allow you to say “trigger only once”.

M

On Fri, Jul 26, 2019 at 1:47 PM Dustin Richmond notifications@github.com wrote:

CUDA, SPMD, etc. CUDA is just the worst, since it freezes/unfreezes tiles repeatedly

On Jul 26, 2019, at 1:46 PM, taylor-bsg notifications@github.com wrote:

I see — you see these every time a CUDA kernel is launched?

On Thu, Jul 25, 2019 at 10:05 AM Dustin Richmond < notifications@github.com> wrote:

I'm referring to this in the Manycore (and a few other examples). These are useful -- for example Borna and I used them to debug -- but they're not clogging the simulation output for CUDA programs. My concern is that these statements are actually slowing down large scale simulation like they would in a C/C++ program.

https://github.com/bespoke-silicon-group/bsg_manycore/blob/64858dd07eaa4b7f8a767b89edc073947bb60bd6/v/vanilla_bean/network_rx.v#L330

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub < https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=AEFG5AGBRTNCIMDUN76PYMLQBHMOBA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD22DOBQ#issuecomment-515127046 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AEFG5ADXNYZOCTG533XEHHTQBHMOBANCNFSM4IG37MJQ

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=ABQVI5GB4SBR2TPHQ4INUGLQBNPCVA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD25VQMI#issuecomment-515594289>, or mute the thread < https://github.com/notifications/unsubscribe-auth/ABQVI5BQSXUOWQ5IF37XPH3QBNPCVANCNFSM4IG37MJQ .

— You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=AEFG5AHJXXCMUPQSFYUFIDTQBNPGXA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD25VSQI#issuecomment-515594561, or mute the thread https://github.com/notifications/unsubscribe-auth/AEFG5ADLXHG6UPPMGR6PISLQBNPGXANCNFSM4IG37MJQ .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=ABQVI5GWDIYIH22NKRMHG2LQBNROVA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD25W3CI#issuecomment-515599753, or mute the thread https://github.com/notifications/unsubscribe-auth/ABQVI5GMAD3GNQ7T7CVYCCDQBNROVANCNFSM4IG37MJQ.

taylor-bsg commented 4 years ago

In the context of BaseJump STL (and maybe bsg manycore), I am starting to thing that it makes sense to have a centralized package which controls the verbosity of printing of things. Then you go to one place to control verbosity. One goal would be to make to sure that people can use BaseJump STL files piecemeal.

dpetrisko commented 4 years ago

The interface being something like +define+BASEJUMP_LOG_LEVEL=DEBUG ?

taylor-bsg commented 4 years ago

More like a SystemVerilog package that allows you to control the kinds of things that the simulator reports via variables or functions.

On Fri, Dec 13, 2019 at 9:33 PM Dan Petrisko notifications@github.com wrote:

The interface being something like +define+BASEJUMP_LOG_LEVEL=DEBUG ?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=AEFG5ADUYFDCMCNVI5HT2BTQYRV3HA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG325WI#issuecomment-565685977, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFG5ABDF54YYAA5OCLXKSLQYRV3HANCNFSM4IG37MJQ .

dpetrisko commented 4 years ago

Oh, I see. The UVM-style approach is to have both a “SEVERITY” which is associated with each print function and “VERBOSITY” which is a global, per build/run setting.

http://cluelogic.com/2015/05/uvm-tutorial-for-candy-lovers-reporting-verbosity/

Could certainly imagine using parameters, macros, functions, whatever for something like this. Or adding additional flags to filter further.

One goal would be to make to sure that people can use BaseJump STL files piecemeal.

What do you mean by this?

taylor-bsg commented 4 years ago

i.e. you should be able to use some modules from BaseJump STL without having to include a lot of unrelated stuff.

On Sat, Dec 14, 2019 at 11:16 AM Dan Petrisko notifications@github.com wrote:

Oh, I see. The UVM-style approach is to have both a “SEVERITY” which is associated with each print function and “VERBOSITY” which is a global, per build/run setting.

http://cluelogic.com/2015/05/uvm-tutorial-for-candy-lovers-reporting-verbosity/

Could certainly imagine using parameters, macros, functions, whatever for something like this.

One goal would be to make to sure that people can use BaseJump STL files piecemeal.

What do you mean by this?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/bespoke-silicon-group/basejump_stl/issues/115?email_source=notifications&email_token=AEFG5AEL7HJLSCT4JQPFYZDQYUWJJA5CNFSM4IG37MJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG4JFIQ#issuecomment-565744290, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFG5AHCVVX7Q4KJWPSGRY3QYUWJJANCNFSM4IG37MJQ .