Use of functions and automatic

[GregAC]

I don't think we currently have any guidance on the use of functions, in particular I think this is important for synthesizable RTL. DV code may want different guidelines or be happy to have none. I'm purely considering synthesizable RTL here.

A few questions to answer:

1. Do we allow use of functions at all in synthesisable RTL?
2. Do we allow use of local variables within functions?
3. Do we mandate use of 'automatic' in all function definitions

I think our answers should be, yes we allow functions, that can use local variables and must always be declared automatic. We probably need some details around how local variables can be used within functions

Clearly functions are useful so saying no functions at all seems very limiting. Allowing local variables though can open up problematic behaviour, e.g.

function [3:0] do_a_thing(input [3:0] foo);
  logic [3:0] state = 0;

  if (foo == ConstantValue) begin
    state = state + 1;
    return state;
  end else begin
    return foo;
  end
endfunction

By default state is static so this function has persistent state between calls. This has clear issues and shouldn't be allowed.

You can use the 'automatic' qualifier on a function to remove the default static state-holding behaviour I suspect there's some further gotchas around local variables in functions we need to be wary of.

[imphil]

From my PoV: 3x Yes.

One further thing to figure out: what guidance do we want to give to DPI functions (I don't even know the exact semantics there ...)

[sjgitty]

This is a good subject, @GregAC thanks for bringing it up. We do have some guidance on our side that we could use as a starting point if you want. Here is a snippet.

---

• No assignment in the function body to the input variables
• Explicitly declare functions and tasks defined outside of a class. They must be automatic in synthesizable RTL.
• Explicitly declare storage types for all arguments in function and task arguments lists.

We have a template also, but it looks a lot like yours with the exception of not mixing declaration and assignment; and only allowing inputs, which is the default, so skipping the keyword input. Example:

// Description of the function's behavior, inputs, and outputs
function automatic logic [2:0] foo(logic [2:0] x, logic [2:0] y);
  // Local variables
  logic [2:0] z;
  // Statements
  z = x ^ y;

  // Return
  return z;
endfunction : foo

Looking at our code base, we follow this pretty closely anyway, with a couple of exceptions.

[sriyerg]

Agree with allowing use of functions in synthesizable RTL. However there are some gotchas though on where the functions can be invoked. For example, the functions defined in ibex_tracer module invoked in always_comb block causes several warnings to be thrown:

Warning-[IDTS] Ignoring dynamic type sensitivity
../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv, 395
"mnemonic"
  Dynamic types used in always_comb, always_latch, always (@*) will be ignored
  for the inferred sensitivity list. 

  Declared at: ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv, 394
  Originating call: ../src/lowrisc_ibex_ibex_tracer_0.1/rtl/ibex_tracer.sv, 
  718

Need to investigate whether this is benign or an issue that needs fixing.

We do have similar guidance on functions in our DV Style guide (but its only limited to functions defined in packages) - https://github.com/lowRISC/style-guides/pull/22/files

We also have added some guidance on DPI-C functions in there.

[imphil]

I think we're good to go with this one, mind doing a PR with proposed wording @GregAC?

(I've filed https://github.com/lowRISC/ibex/issues/619 for the sensitivity warnings. Thanks @sriyerg for the reminder.)

[GregAC]

Happy to do the PR, thanks for the input everyone

[msfschaffner]

Sorry for being late to the party here. 3x times yes as well from my side. I just wanted to add that lint would flag non-automatic functions in RTL. For example, if I remove automatic from one of the functions in aes_pkg.sv, the following error is thrown:

E   AUTOMATIC:   aes_pkg.sv:69   Function 'aes_mul2' is not declared as 'automatic'