Refactor `haveExt()` & `sys_enable_ext()` functions

[Timmmm]

I think the haveExtension() functions are a bit confusingly named. It's unclear whether they mean that the CPU supports the extension, or if it is currently enabled (which are not necessarily the same if you have writeable misa). I believe the answer is the latter, and there are a few sys_enable_extension() functions which actually dictate whether the extension is supported. That function only exists for some extensions though.

This is confusing. I think we should do the following:

1. Rename all haveExtension() functions to extensionEnabled().
2. Rename all sys_enable_extension() functions to sys_support_extension() or sys_extension_supported().
3. Add sys_extension_supported() functions corresponding to all the haveExtension() functions, so they are 1:1. Initially we can just hard-code the result rather than adding CLI arguments.
4. Update legalize_misa() to use the sys_extension_supported() functions (even if they are hard-coded to true).

[arichardson]

I agree that this separation makes a lot of sense. Not sure it really matters but I wonder if we can use a common prefix for the enabled checks instead of a suffix? enable<Ext> isn't particularly nice but at least more explicit that haveXXX. I guess we could also use something like canUse<Ext>, is<Ext>Enabled

[Timmmm]

Yeah I was thinking exactly the same thing - common prefix is definitely nicer but I couldn't think of a good way to word it that makes sense.

[jrtc27]

I'd rather see the extensions become a (scattered) enum that gets passed to haveExt/isExtEnabled/whatever (scattered) functions.

[jordancarlin]

Now that #758 is merged, this is worth revisiting.

We have the scattered extensionEnabled(Ext_*) function for when an extension is currently enabled. I think it makes sense to replace all of the sys_enable_extension() functions with another scattered function (maybe extensionSupported(Ext_*)) that returns if an extension is supported by the current configuration. Each extension would have a clause in this scattered function that points to the config file (function clause extensionSupported(Ext_Zaamo) = config extensions.Zaamo.enabled).

@Alasdair @Timmmm @pmundkur, any thoughts on this?

[Timmmm]

I think we have to be really careful about the language since this has the potential to be very confusing. Also RISC-V has a special "implemented" meaning that we might want to use... or avoid:

For a given RISC-V execution environment, an instruction, extension, or other feature of the RISC-V ISA is ordinarily judged to be implemented or not by the observable execution behavior in that environment. For example, the F extension is said to be implemented for an execution environment if and only if the instructions that the RISC-V Unprivileged ISA defines for F execute as specified.

With this definition of implemented, disabling an extension by clearing its bit in misa results in the extension being considered not implemented in M-mode. For example, setting misa.F=0 results in the F extension being not implemented for M-mode, because the F extension’s instructions will not act as the Unprivileged ISA requires but may instead raise an illegal-instruction exception.

Defining the term implemented based strictly on the observable behavior might conflict with other common understandings of the same word. In particular, although common usage may allow for the combination "implemented but disabled," in this document it is considered a contradiction of terms, because disabled implies execution will not behave as required for the feature to be considered implemented. In the same vein, "implemented and enabled" is redundant here; "implemented" suffices.

[jordancarlin]

Seems like the special definition of "implemented" lines up with what we are currently calling "enabled" with our extensionEnabled function. Not sure if we want to change that.

I think "supported" is fairly unambiguous, but am open to suggestions if anyone has a better idea. We definitely need to migrate away from both of these being referred to as "enabled" though, which is what we currently have.

[arichardson]

I think supported/implemented/enabled is going always going to be confusing to new readers of the model. Maybe we need something very explicit for extensions that are supported by the model but dynamically turned off due to some csr, etc.

Maybe something very explicit like canBeEnabled vs. extensionEnabled?

[Timmmm]

Something like currentlyEnabled(...) & cpuSupports(...)?

[pmundkur]

I like the use of extensionSupported(Ext_*) to indicate a static option coming from the config file, and extensionEnabled(.) to indicate whether it is dynamically usable.

However,

function clause extensionSupported(Ext_Zaamo) = config extensions.Zaamo.enabled

is not ideal; I'd prefer

function clause extensionSupported(Ext_Zaamo) = config extensions.Zaamo.supported

which involves tweaking the json schema. Since we're still early in this config system, we could do it before the schema makes it into a release.

[arichardson]

I think changing the schema makes sense, but I do still worry that "supported" and "enabled" are so similar that it's easy to accidentally use the wrong one when adding new features since supported and enabled can both be interpreted as "currently supported".

[jordancarlin]

How about extensions.Zaamo.supported for the config file, and currentlyEnabled(...)/cpuSupports(...) for the two different scattered functions?