ISA extension spec probably should not require intrinsics

[marcfedorow]

Neither bitmanip nor crypto specks require intrinsics. This is probably not an argument against intrinsics: most of bitmanip instructions are emitted by compiler and crypto finds its usage in such libraries as OpenSSL (where generics are already written).

The main argument against intrinsics -- generic code could probably be optimized better than one with intrinsics. I.e.

#if __riscv_zpn
asm("...");
#else
generic();
#endif

is presumably better than multiple intrinsic calls.

Intrinsics are about software but not about instruction set architecture -- so they should be probably developing in riscv-software task group (as it was discussed about crypto intrinsics at the latest meeting).

[jrtc27]

Whilst I agree that the global is ugly, I disagree that your proposal is a better fit for the architectural specification. Architecturally, illegal instruction is just another exception, and so should follow the exact same path as other exceptions.

Which I think means what you really want is that all exception cases are RETIRE_FAIL(exception_details), with the top level then being responsible for the exception handling, passing on the extra state needed, including instbits for illegal instructions.

[Timmmm]

Good call, that does sound better and I guess would let you treat other exceptions in the same way (I'll have to double check the code though). I don't think exception_details needs to include the instructions bits though since they are already available at the point in step() where it's handled (and they'd be a bit awkward to get in execute() since the instruction has already been decoded).

[jrtc27]

Good call, that does sound better and I guess would let you treat other exceptions in the same way (I'll have to double check the code though). I don't think exception_details needs to include the instructions bits though since they are already available at the point in step() where it's handled (and they'd be a bit awkward to get in execute() since the instruction has already been decoded).

Sorry I should have been clearer: instbits would be supplied to handle_exception to use in the case that the execute clause returned Exc_Illegal_Inst (names entirely made up, any semblance to real names is coincidental). Or, to put it another way: yes, that's what I meant, I just failed to clearly communicate it.

[martinberger]

I'm also in favour of extending the Retired data type.

There is another important case: Sdtrig debug triggers (https://github.com/riscv/riscv-debug-spec). One way to implement is to centralise trigger handling uniformly in step. But if you get a trigger for example of a load address, that trigger is found in the execute clause of the relevant load instruction. We don't want to handle the trigger in all relevant execute clauses, so need to pass this information back to step where it can be handled uniformly. (And where we need to handle triggers on the PC anyway.) For this purpose a RETIRE_TRIGGER(...) would be neater than yet another set of global, but non architectural registers.

Extending Retired does not require changing all the existing code that returns RETIRE_SUCCESS or RETIRE_FAIL, so it's not an intrusive change.