Bool parameters which default to enabled can't be disabled

[rswarbrick]

If a core file specifies a boolean parameter like this (taken from Ibex):

  RV32M:
    datatype: bool
    paramtype: vlogparam
    default: true
    description: Enable the M ISA extension (hardware multiply/divide)

then the argument parser for fusesoc run gets a --RV32M argument which takes no arguments and switches the flag on. This is very clean and nice!

Of course, the example above has default: true, so passing --RV32M has no effect. However, there's no --no-RV32M flag to turn the feature off. Oh no!

[olofk]

Yeah, sorry about that. I know about it but just have never been able to prioritize it high enough. I see you switched to using ints instead. I must shamefully confess that I do the same thing when I have something that should be enabled by default but optionally disabled. Keeping this open as it's something that should be fixed. I also considered automatically adding a corresponding --no-option to each --option, but perhaps it's easier to add support for --option=false instead. Not sure really, but we'll get it done at some point

[olofk]

I fixed this now on the FuseSoC side, but it turns out to cause some problems in Edalize. The issue is really a philosophical one because the boolean parameters are really tristate. They can be 1) true, 2) false or 3) not exist at all. Up until now Edalize have only received boolean parameters with value set to True. Otherwise, the parameter was never passed from FuseSoC. With the propsed change, we probably need to support passing booleans with false values also. This leads to a question. For a false boolean parameter, should we a) treat it as a parameter with value false, and cast to 0 for e.g. vlogdefine, plusarg and vlogparam which don't support true bools b) Remove it completely when calling the tools

I think b would be closest to what we have now and is probably what most people want, but I'm not sure. Any input is appreciated @rswarbrick... @imphil..anyone?

[imphil]

For Verilog defines with a bool type I'd expect the define to be either set or not set; the typical check done is ifdef/ifndef. See https://github.com/olofk/fusesoc/issues/263.

For plusargs with boolean types I'd do the same, true == plusarg is present, false == plusarg is missing. That's in line with testing done with $test$plusargs().

For vlogdefines and plusargs, "pass nothing" implicitly means false.

The tricky case are parameters, where we have the option of "pass true", "pass false", and "pass nothing and use the default specified in the HDL (not in fusesoc!)". True and false are easy. "Pass nothing" should IMO mean nothing in fusesoc sets or forces the parameter -- not a default: true, not a param=true, not a command line option. We simply treat this parameter as if it would have been not defined in fusesoc and don't include it in the EDAM file.

[olofk]

I would need to write down all the cases to understand the implications. Feelse like there are a suprisingly large number of corner cases to be aware of.