Bitwise "not" operator does not have intended effect on ispresent

[nachumg]

Given this code:

  regfile a #(longint unsigned NUM_REGS = 1, bit reg_present = 1){
    regfile b {
      reg some_reg { field {} nested_field;} some_reg[NUM_REGS];
      reg other_reg { field {} nested_field; ispresent = ~reg_present;} other_reg;
    } b;
  };

When the type is instantiated with default params the value of

ispresent = ~reg_present
is (2^64 - 1) which is not false as clearly intended by the author.

The fix is to instead use

ispresent = !reg_present

My question: is this a feature or a bug? The standard is not clear on whether bit-type param fields are supposed to be full-width (64-bits) or minimum-width required to hold the default value (1-bit).

Thanks.

[nachumg]

Correction: the value of ~reg_present in the previous example is 0xfffffffffffffffe

[amykyta3]

This is strangely intentional.

The way the SystemRDL spec defines integer datatypes is somewhat odd. longint unsigned is clearly defined as a 64-bit unsigned integer. The bit datatype borrows from SystemVerilog, but in SystemRDL there is no way to specify a width for it explicitly. ie: RDL does not allow you to declare bit [7:0]. At the same time, SystemRDL interprets the bit type as something that can hold any unsigned integer, of any bit-length. Because of this, the compiler can only infer the width based on context.

Since an integer literal of just 1 does not specify a width, the compiler assumes a width of 64-bits. From here it should be clear that inverting 64'b1 using a bitwise NOT results in 64'hfffffffffffffffe. Once cast to a boolean type, this becomes true

Using reg_present = 1'b1 would avoid this quirk, but more appropriate would to use a boolean type instead:

regfile a #(longint unsigned NUM_REGS = 1, boolean reg_present = true){

[nachumg]

Thanks.

Should there be a compile-time error if ispresent is assigned an integer that is not zero or 1 ?

[amykyta3]

I don't think so as that opens a whole can of worms of annoying behavior in places where implicit casting would be desired. SystemRDL effectively defers to SystemVerilog in terms of how expressions and casting works - in SystemVerilog, such an assignment would not be an error so I feel it would be inappropriate to do the same here.

For a moment, I considered making RDL un-sized literal representations be automatically pruned down to their minimum required bit width, such that 1 is 1-bit wide, 15 is 4-bits wide, etc.. Unfortunately this ends up being even more problematic: Since RDL also defers to the SystemVerilog spec regarding how operators resolve widths of expressions, auto-sizing integer literals like this would result in some even more surprising behavior, like 1 + 1 resolving to a value of 0 in some expression contexts.

[nachumg]

How about a warning, and a compiler option to make it an error?

For that matter, could each warning get its own number (e.g. W101), and allow the user to decide which warnings are really an error?

[amykyta3]

I'd be open to considering this. I plan to massively refactor how the compiler message system works (#168) to allow for such mechanisms, so this would likely not be a feature until that happens.

[nachumg]

Seems like there already is a list of warnings?

systemrdl-compiler/src/systemrdl/compiler.py

Line 477 in c62b079

self.chk_missing_reset = self.chk_flag_severity(warnings.MISSING_RESET, w_flags, e_flags)

Can this be added to that list?