Common field reference table

[drom]

Currently, the named fields are belong to each instruction and don't carry instruction based semantics.

Here is example of addi instruction with rs1, rd, imm

addi:
  long_name: Add immediate
  description: Add an immediate to the value in rs1, and store the result in rd
  definedBy: I
  assembly: xd, xs1, imm
  encoding:
    match: -----------------000-----0010011
    variables:
    - name: imm
      location: 31-20
    - name: rs1
      location: 19-15
    - name: rd
      location: 11-7

Does it make sense to specify reference from each instruction into common reference table that will carry more detailed information about the field (type, zero/sign extension, addressing mode, source/destination, etc.)
Here is how the table could look like:

https://github.com/drom/riscv/blob/master/lib/fieldo.js

This information could be elaborated inside the instruction when flat form is needed.

addi:
  long_name: Add immediate
  description: Add an immediate to the value in rs1, and store the result in rd
  definedBy: I
  assembly: xd, xs1, imm
  encoding:
    match: -----------------000-----0010011
    variables:
    - {$ref: 'fieldo.json/imm12'}
    - {$ref: 'fieldo.json/rs1'}
    - {$ref: 'fieldo.json/rd'}

Also, looking at the large usage counts of some fields, it might be important to know that some set of instructions share semantics of some field.

[dhower-qc]

That's an interesting suggestion, and it would be great to capture more semantic information without having to parse/analyze the IDL code. I'm 100% on board with getting this information in, but just want to think through the best approach.

Generally, we've tried to keep .yaml files under arch bare-bones to minimize the dependencies needed to consume the data in any programming language ('flat' as you say). That's why we've avoided things like YAML anchors/refs or, in this case, JSON references.

So, what to do?

1. Introduce JSON Reference in .yaml files under arch. We can provide a flat version as a generated artifact with each commit (we should really be doing this anyway).
2. Use JSON Reference in template files that generate .yaml files under arch. We already do something like this in limited cases where there is a lot of repetition (e.g., the pmpaddrN registers, which are generated from pmpadddrN.layout).
3. Don't introduce JSON Reference, and just add the 'flat' info to the existing .yaml files. We could programmatically detect common fields, but it wouldn't be explicit in the data anywhere.

I'm inclined to go the route of option 1, but just worry that it could be another obstacle making UDB adoption less likely. Especially if JSON Reference becomes a requirement for anyone contributing to the DB (e.g., extension authors).

This is a good topic to discuss in a sync-up meeting. In the meantime, I'm curious what your thoughts are.

** Side note **

This data is related to instruction formats (I-type, R-type, etc.), which has come up a few times since it's not currently captured by the database. This was initially intentional -- in my corner of the world, we generally don't care about them, especially since it gets ugly quick with things like the C extension. However, compiler folks like @apazos have been educating me on why it's important to capture it, e.g., for linker relaxation information. It seems like this would be right place to figure out how to integrate the format information.

[drom]

You already have some implicit references from one file to another, like definedBy: I. Implicit links require custom sideband knowledge about the ways to reason components together.
Explicit JSON/YAML references is an explicit composition mechanism that is still "human readable/writable" but allows declarative, low-overhead, language agnostic automatic resolution.
IMHO we should use $ref

[drom]

This data is related to instruction formats (I-type, R-type, etc.), which has come up a few times since it's not currently captured by the database.

Yes, canonical '*-type' format is an very high level abstraction that is already broken in many places. I don't see much value caring it as is through the machine readable spec. On other hand, the unique set of field types constitutes specific type format. for example:

variables:
    - {$ref: 'fieldo.json/imm12'}
    - {$ref: 'fieldo.json/rs1'}
    - {$ref: 'fieldo.json/rd'}

We can call it {rd,rs1,imm12}-type format.

All C-type formats just automatically get specialized forms:

https://github.com/drom/riscv/blob/master/lib/fieldo.js#L63

[AFOliveira]

As a follow up on earlier discussion, I'll take the first step towards this and turn the variables name into their own nicknames, if no one has anything against it. After those nicknames are correct, I guess moving to a json schema pass would be just find/replace, correct?

@drom @dhower-qc Do you think that we should keep values like imm12hi, imm12lo separated on the variable list? It seems quite odd because no matter how the encoding is done, it actually is only one variable.

[dhower-qc]

As a follow up on earlier discussion, I'll take the first step towards this and turn the variables name into their own nicknames, if no one has anything against it.

Excellent

After those nicknames are correct, I guess moving to a json schema pass would be just find/replace, correct?

Take a look at #228. I've updated the schema to handle $mref from an instruction variable. (this is different than $ref, which according to JSON Reference should ignore any other fields, which isn't what we want here). I kept the old schema around, too, so we don't have to convert everything now.

@drom @dhower-qc Do you think that we should keep values like imm12hi, imm12lo separated on the variable list? It seems quite odd because no matter how the encoding is done, it actually is only one variable.

I would prefer we merge fields like that into one so that we maintain a 1:1 mapping between the variable names and the type information.

[AFOliveira]

@drom @dhower-qc Do you think that we should keep values like imm12hi, imm12lo separated on the variable list? It seems quite odd because no matter how the encoding is done, it actually is only one variable.

I would prefer we merge fields like that into one so that we maintain a 1:1 mapping between the variable names and the type information

I agree with your view here, problem is that riscv-opcodes has fields like imm12 and imm12hi, imm12lo which mean different fields.
I think we have at least two approaches to move forward on this:

• We define new nicknames that allow for all the previous encodings, this has the upside of being kind of easy due to being a direct renaming from riscv-opcodes
• We define new nicknames, but ally them to Instruction type, i.e. U-Type, B-Type and use this encondings types to ensure that we never miss any field. This has the upside of using ratified and generic description that anyone would easily understand when reading and writing, and would allow for yet again the speck checking upon itself, due to each instruction type having specific fields.

I am more inclined to the second one, because I think it is more sustainable in the future, but would like to hear your takes on this.

[dhower-qc]

I agree; the second approach seems better long-term.

[ThinkOpenly]

FWIW, I'm not a big fan of associating too much meaning to the names of the assembly operands. Looking at what riscv-opcodes has shows that it gets messy. The operand names aren't architecturally significant. You could have an operand called "bimm12hi", or you could call it "X". The important information is the operand order, what the operand represents, and how it is encoded. The order is already present in the "assembly" field, the representation could be included in the "flat" YAML (GPR register file index, although the "types" would then need to be defined somewhere), and the encoding is already present.

(In some ways, it would be nice to use more semantically meaningful names for the operands, like "offset", "addend1" and "addend2", "numerator" and "denominator", but perhaps that ship has sailed.)

[AFOliveira]

I agreed and defended exactly your point on a previous meeting, however @drom's point to which I also agreed was that

important information is the operand order, what the operand represents, and how it is encoded

was going to get duplicated if we didn't nickname. So what we're trying to do is nickname but associate that nickname to a data structure that has all the parameters that actually define that operand.

[ThinkOpenly]

important information is the operand order, what the operand represents, and how it is encoded

was going to get duplicated if we didn't nickname. So what we're trying to do is nickname but associate that nickname to a data structure that has all the parameters that actually define that operand.

Understood, but I'm more in favor of establishing "types" for the operands, rather than depending on nicknames. While similar, there is a distinction: one issue, IMHO, with nicknames is that it tries to encapsulate both the type and the encoding, as in the example in your comment #203 (comment):

problem is that riscv-opcodes has fields like imm12 and imm12hi, imm12lo which mean different fields.

I propose avoiding the use of riscv-opcodes as a model. Let the operands stand on their own, and let the encoding either be specified (duplicated) with the instruction, or we wade into formats.

We are going to need to include instruction formats at some point. Downstream projects need that information, like binutils ("INSN_CLASS"):

/* All RISC-V instructions belong to at least one of these classes.  */
enum riscv_insn_class
{
  INSN_CLASS_NONE,

  INSN_CLASS_I,
  INSN_CLASS_C,
  INSN_CLASS_M,
  INSN_CLASS_F,
  INSN_CLASS_D,
  INSN_CLASS_Q,
  INSN_CLASS_F_AND_C,
  INSN_CLASS_D_AND_C,

{"addi",        0, INSN_CLASS_I, "d,s,j",     MATCH_ADDI, MASK_ADDI, match_opcode, 0 },

QEMU:

# Formats 32:
@r       .......   ..... ..... ... ..... ....... &r                %rs2 %rs1 %rd
@i       ............    ..... ... ..... ....... &i      imm=%imm_i     %rs1 %rd
@b       .......   ..... ..... ... ..... ....... &b      imm=%imm_b %rs2 %rs1
@s       .......   ..... ..... ... ..... ....... &s      imm=%imm_s %rs2 %rs1
@u       ....................      ..... ....... &u      imm=%imm_u          %rd
@j       ....................      ..... ....... &j      imm=%imm_j          %rd

[dhower-qc]

Check out https://github.com/riscv-software-src/riscv-unified-db/blob/main/arch/inst/I/addi.yaml

That's an example of using the recently added $inherits feature. It should allow us to 'inherit' operand types while keeping the name independent. I agree with Paul that we want an independent name to, for example, distinguish imm vs offset.

[AFOliveira]

That's an example of using the recently added $inherits feature

My question might seem dumb, but isn't that just shifiting the nicknames to the end of the inherit?
$inherits: ../../common/inst_variable_types.yaml#/**itype_imm**

[ThinkOpenly]

That's an example of using the recently added $inherits feature

My question might seem dumb, but isn't that just shifting the nicknames to the end of the inherit?

I had the same impression. Given that the "location" is pretty closely tied to the instruction format, is it possible to inherit all of the operands and their locations based on the instruction format? (Showing my JSON ignorance here.) Something like:

  encoding:
    match: -----------------000-----0010011
    variables:
      $inherits: ../../common/inst_formats.yaml#/itype

and the "itype" stanza in inst_formats.yaml would have all of the variable fields:

  itype:
    - name: imm
      location: 31-20
    - name: rs1
      location: 19-15
    - name: rd
      location: 11-7