Add reserved_behaviour.AMOCAS_odd_registers config

[challenger1024]

Add a configuration option to control the behaviour when executing AMOCAS instructions that are reserved due to the use of odd register indices.

This is the first setting we've added to control reserved behaviour. It allows two options: a fatal error that stops the simulation, or to treat it as an illegal instruction (the default choice).

See #775

[Timmmm]

Nice! I think maybe it would make more sense to have a fine grained config where the behaviour for each kind of reserved behaviour can be controlled. The spec isn't very clear about the semantics of reserved behaviour in general (sometimes it is UNSPECIFIED but not always), so I think it's easiest if we're explicit about it.

Also we probably want an enum rather than a bool to allow more possible behaviour options in future. Something like:

"reserved_behavior": {
  "nonexistent_csr": "illegal"/"read_only_zero"/"exception",
"amocas_odd_registers": "illegal"/"exception",
"reserved_fence": ...

[Timmmm]

Also I reckon we should add another function to use for this instead of internal_error, and probably another exception type.

[jordancarlin]

Beyond @Timmmm's comments, I think we should also avoid using the term "strict" to describe the mode that terminates the simulation because of potential confusion with the Ssstrict extension (which requires most reserved behavior to throw an exception).

[challenger1024]

Changes Introduced in the new Commit

• Updated the configuration item in config.json.in to define reserved_behaviour as an enumeration type. At present, it includes only one entry — amocas_odd_registers , which can take the following values:
  • illegal : returns Illegal_Instruction
  • ignore : ignores amocas_odd_registers and continues executing the remaining part of the AMO function
• Replaced the call to internal_error in model/extensions/A/zaamo_insts.sail with return Illegal_Instruction()

Questions for Clarification

• Would it be appropriate to introduce a dedicated exception type for reserved_behaviour?For instance, when config base.reserved_behaviour.amocas_odd_registers = "exception", we could return a specific exception type, such as:

  union Reserved_Behaviour {
    Amocas_odd_registers : unit,
    ...
  }

• Should we implement a unified handler function to manage exceptions raised by reserved behaviours,or would it be preferable to define a dedicated handler for each reserved behaviour individually, for example:

  function handle_reserved_behaviour(reserved_behaviour : Reserved_Behaviour) {
  
  }

@Timmmm

[Timmmm]

Would it be appropriate to introduce a dedicated exception type for reserved_behaviour?For instance, when config base.reserved_behaviour.amocas_odd_registers = "exception", we could return a specific exception type, such as:

Sounds like a good idea to me!

Should we implement a unified handler function to manage exceptions raised by reserved behaviours,or would it be preferable to define a dedicated handler for each reserved behaviour individually, for example:

Yeah I think so, like the existing internal_error function.

[challenger1024]

After reviewing the sail-riscv source code, I found that when the registers
are odd-numbered, amo_encoding_valid() already returns false for AMOCAS.D/Q.
As a result, the model treats these encodings as illegal instructions since
the AMO mapping becomes invalid. Therefore, I focused only on handling the
case when the configuration is set to "exit", defining how the model should
terminate in that situation.
latest commit ,Move odd-register check for AMOCAS.D/Q into amo_encoding_valid()
@Timmmm

[Timmmm]

Some code fixes needed but the idea looks good to me now.

I'm curious what the motivation for adding the flag for this particular reserved behaviour is, or was it just randomly picked from the list in #775 ?

[Timmmm]

@Alasdair does Sail guarantee short-circuit boolean operator semantics? Presumably yes; just thought I'd double check.

[challenger1024]

In the latest commit, I added the ReservedBehaviourPolicy enum and the reserved_behaviour function.
First, to answer your question above: selecting an AMOCAS odd-number register is currently random, but if possible, I would like to extend the configuration to include other reserved behaviours mentioned in #775 as well.
In addition, I have a few questions:

1. I’m not entirely sure whether placing ReservedBehaviourPolicy in types.sail is the best choice — is there a more appropriate location?
2. Is using the name ReservedBehaviourPolicy suitable, or would you suggest a better alternative?
3. For the enum members of ReservedBehaviourPolicy, should I rename them to RB_Exit or RB_Illegal to avoid potential naming conflicts with other occurrences of Illegal?
   @Timmmm

[Timmmm]

LGTM, just a few minor tweaks.

Also we might want to use the barbarian American spelling of behaviour without the u. They do have nukes...

[challenger1024]

last commit,Change behaviour to behavior, change Exit to Fatal, and make other minor adjustments

@Timmmm

[Timmmm]

LGTM, thanks!

[github-actions]

Test Results

2 115 tests   2 115 ✅  17m 54s ⏱️
    1 suites      0 💤
    1 files        0 ❌

Results for commit 84b34f5.

♻️ This comment has been updated with latest results.

[challenger1024]

last commit, Remove previously ignored redundant function declaration: reserved_behavior_exit
@Timmmm

[challenger1024]

last commit, change enum name ReservedBehaviorPolicy to AmocasOddRegisterReservedBehavior.
@Timmmm

[challenger1024]

Hello, I’ve tried adding the configuration option into platform_config.sail, but I ran into some issues.
My code looks like this:

let amocas_odd_register_reserved_behavior : AmocasOddRegisterReservedBehavior = config base.reserved_behavior.amocas_odd_register

However, since the AmocasOddRegisterReservedBehavior enum is defined in types.sail, and in model/riscv.sail_project the types.sail file is listed after platform_config.sail, platform_config.sail cannot find AmocasOddRegisterReservedBehavior.
Do you have any suggestions for how to resolve this issue?
Should I move AmocasOddRegisterReservedBehavior to another file, and if so, where would be the best place to put it?
@pmundkur

[Timmmm]

Should I move AmocasOddRegisterReservedBehavior to another file, and if so, where would be the best place to put it?

I think it's probably fine to move it to platform_config.sail.

[pmundkur]

Yeah, moving to platform_config is probably okay for now. We should refactor types.sail at some point. It depends on extensions now due to Ext_H which is not good for a file called types.

[challenger1024]

last commit,
Move AmocasOddRegisterReservedBehavior from types.sail to platform_config.sail, and add the variable amocas_odd_register_reserved_behavior in platform_config.sail. Removed extra blank lines in errors.sail.

[Timmmm]

LGTM

[pmundkur]

I added some minor cleanup and rebased.