Update Zimop instructions #455
Conversation
I sure hope not. I suspect they'll mostly be used by implementations as HINT-like instructions. Normal software is discouraged from ("should not" be) using them.
Good question that I can't speak to. It's not obvious to me. I suspect that if one wanted to override these encodings in an extension, that extension should preclude ("conflicts") Zimop. This is the first time I've looked at these files, but, (this is not in your changes) the "assembly" field looks wrong to me: assembly: mop_r_t_30, mop_r_t_27_26, mop_r_t_21_20, xd, xs1The "assembly" field is described: "assembly": {
"type": "string",
"description": "Assembly format of the instruction. Can use decode variables"
},That says to me that it should represent the assembly format of the instruction ;-), which is really Also, the specific realizations of |
| @@ -3,9 +3,11 @@ | |||
| $schema: "inst_schema.json#" | |||
| kind: instruction | |||
| name: mop.r.n | |||
| long_name: No synopsis available. | |||
| long_name: May be an OPeration (single register). | |||
There was a problem hiding this comment.
nit: Suggest "May-be-operation (1 source register)"
Unfortunately, the ISA doesn't provide names, but the chapter title is "'Zimop' Extension for May-Be-Operations, Version 1.0".
If this single definition eventually gets expanded to 32 instruction definitions, then you could use "May-Be-Operation N (1 source register)" (or something like that).
| @@ -3,9 +3,11 @@ | |||
| $schema: "inst_schema.json#" | |||
| kind: instruction | |||
| name: mop.rr.n | |||
| long_name: No synopsis available. | |||
| long_name: May be an OPeration (2 registers). | |||
There was a problem hiding this comment.
similarly, I suggest "May-be-operation (N) (2 source registers)".
| The Zimop extension defines 32 MOP instructions named MOP.R.n, where n is an integer between 0 | ||
| and 31, inclusive. Unless redefined by another extension, these instructions simply write 0 to X[rd]. | ||
| Their encoding allows future extensions to define them to read X[rs1], as well as write X[rd]. |
There was a problem hiding this comment.
If this single definition eventually gets expanded to 32 instruction definitions, then this text should probably change to something like:
These instructions simply write 0 to X[rd].
May-be-operations (MOPs) are designed to be redefined by later extensions to perform some other action. The encoding allows future extensions to define it to read X[rs1], as well as write X[rd].
There was a problem hiding this comment.
I don't know if we should remove the part that explains that this is 32 instructions technically, especially if generating the spec without expanding it to 32 different definitions
| The Zimop extension defines 8 MOP instructions named MOP.RR.n, where n is an integer between 0 | ||
| and 7, inclusive. Unless redefined by another extension, these instructions simply write 0 to X[rd]. | ||
| Their encoding allows future extensions to define them to read X[rs1] and X[rs2], as well as write X[rd]. |
There was a problem hiding this comment.
Similarly, if this single definition eventually gets expanded to 32 instruction definitions, then this text should probably change to something like:
These instructions simply write 0 to X[rd].
May-be-operations (MOPs) are designed to be redefined by later extensions to perform some other action. The encoding allows future extensions to define it to read X[rs1] and X[rs2], as well as write X[rd].
|
First of all, I had this feeling that MOP instructions were an Achilles tendon of my generation of all files in the UDB, so this was a great catch @jmawet, thank you!
I'm pretty sure they don't, but still Zimop is itself ratified, so I guess we gotta handle it anyway.
Yes, the UDB can handle this via the cfgs/arch_overlay folder. No ratified extensions uses it, the only use-cases for now would be vendor-specific, so I think it is enough to have this funcionality as part of a configuration-specific folder.
To answer @ThinkOpenly: yes, the assembly is wrong and it is my fault, the script that generated them clearly had a bug. On separating the instructions in multiple files: if we follow UDB pattern so far, they should all have its own file. We could find a way to generate a middle-end that would multiply them -this is what riscv-opcodes does- and their point is to avoid duplication. However, we are already duplicating the V extensions instructions that follow under a similar category, so I think we should just keep the methodology here. If anyone is happy to contribute that, I think it would be great, otherwise I'll eventually get to it :) |
The shadow stack (Zicfiss) and landing pad (Zicfilp) extensions (chapter 35 of the unpriv ISA manual) use the MOP instructions. This is a ratified standard extension, so I don't think the arch_overlay option is sufficient. |
I guess my wrong answer didn't last for long :). Thank you for the input, Jordan! Then, I'm not sure how to add them. RISC-V opcodes specifies them as pseudo, but I believe it is misleading, as the spec clearly states them as "instructions". It's not clear to me what is the relation between them in UDB perspective. I see two ways we could handle this, but there are probably more and better:
What do you guys think? Any other ideas on how to act on this? |
|
Good discussion! My view (and one that works with the generated ISS decoder): The mops are not pseudo-instructions. They consume a new encoding that was previously reserved. When an instruction overrides a mop, it can be encoded as a "hint" (bad name in this context) in UDB. Recall from our prior discussions that "hint" is an instruction that reuses an existing opcode but does something different than the original (in contrast with a "pseudoinstruction" that just gives a new name to the exact same behavior). In the cpp_hart branch, I have already modified, e.g., hints:
- { $ref: inst/Zicfilp/sspopchk.x1.yaml# }
- { $ref: inst/Zicfilp/sspopchk.x5.yaml# }
- { $ref: inst/Zicfilp/ssrdp.yaml# }The hint instructions specify their own encoding in their YAML file, which will be some overlap of the base instruction (I think I even check this invariant). Hints take decode priority over the base instruction, so there is no ambiguity/conflict with the encoding. And when Zicfilp is not implemented, the hint opcodes revert to I think this all works, with the caveat that we could use a better name for "hint" since these particular instructions are clearly more than hints. |
|
I didn’t have all the context on how hints were implemented, but that seems like a very natural solution to this. In terms of naming, maybe something as simple as |
This is what I was thinking as well. I only wondered if
A hint instruction which overrides an instruction encoding which is otherwise a no-op is arguably still an override, IMHO, so I think |
True enough and I think that would be fine if we want to avoid the confusion of duplicate keywords with the same meaning. |
Updated instructions for Zimop.
As they are pseudo instructions, I wrote the operation as the default X[rd]=0 (Per page 59 of the spec: Unless redefined by another extension, these instructions simply write 0 to x[rd].). Not sure if any currently ratified extensions use any of the MOP instructions, but if any were to be added, can this unified-db handle that? Could another instruction call this one as a "template" of sorts and change the operation? Because it's my understanding that MOP (may be instructions) are just placeholder instructions, so being able to take advantage of that in this unified-db could be useful if any future extensions take advantage of it. Just a thought