[otbn] Allow short names for operands in encoding docs
This is just aesthetic, but avoids gigantic names like "OFFSET_1" from
appearing in encoding tables. This improves the layout a bit, and
helps decouple the upper-case "field name" from the lower case
"operand name" in the reader's mind.
Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
diff --git a/hw/ip/otbn/data/base-insns.yml b/hw/ip/otbn/data/base-insns.yml
index 3b3fe66..2437fca 100644
--- a/hw/ip/otbn/data/base-insns.yml
+++ b/hw/ip/otbn/data/base-insns.yml
@@ -239,7 +239,11 @@
- mnemonic: lw
rv32i: true
synopsis: Load Word
- operands: [grd, offset, grs1]
+ operands:
+ - grd
+ - name: offset
+ abbrev: "off"
+ - grs1
syntax: <grd>, <offset>(<grs1>)
encoding:
scheme: I
@@ -262,7 +266,11 @@
- mnemonic: sw
rv32i: true
synopsis: Store Word
- operands: [grs2, offset, grs1]
+ operands:
+ - grs2
+ - name: offset
+ abbrev: "off"
+ - grs1
syntax: <grs2>, <offset>(<grs1>)
encoding:
scheme: S
@@ -289,6 +297,7 @@
- grs2
- &branch-offset-operand
name: offset
+ abbrev: "off"
pc-rel: true
type: simm<<1
straight-line: false
@@ -431,6 +440,7 @@
doc: Name of the GPR containing the number of iterations
- &bodysize-operand
name: bodysize
+ abbrev: sz
type: uimm+1
doc: Number of instructions in the loop body
straight-line: false
diff --git a/hw/ip/otbn/data/bignum-insns.yml b/hw/ip/otbn/data/bignum-insns.yml
index 02d2c3d..b8228d6 100644
--- a/hw/ip/otbn/data/bignum-insns.yml
+++ b/hw/ip/otbn/data/bignum-insns.yml
@@ -16,16 +16,19 @@
doc: Name of the second source WDR
- &bn-shift-type-operand
name: shift_type
+ abbrev: st
type: enum(<<, >>)
doc: |
The direction of an optional shift applied to `<wrs2>`.
- &bn-shift-bits-operand
name: shift_bits
+ abbrev: sb
type: uimm5<<3
doc: |
Number of bits by which to shift `<wrs2>`. Defaults to 0.
- &bn-flag-group-operand
name: flag_group
+ abbrev: fg
type: uimm1
doc: Flag group to use. Defaults to 0.
syntax: &bn-add-syntax |
@@ -143,6 +146,7 @@
operands:
- &mulqacc-zero-acc
name: zero_acc
+ abbrev: za
type: option(.Z)
doc: Zero the accumulator before accumulating the multiply result.
- &mulqacc-wrs1
@@ -150,6 +154,7 @@
doc: First source WDR
- &mulqacc-wrs1-qwsel
name: wrs1_qwsel
+ abbrev: q1
type: uimm2
doc: |
Quarter-word select for `<wrs1>`.
@@ -164,6 +169,7 @@
doc: Second source WDR
- &mulqacc-wrs2-qwsel
name: wrs2_qwsel
+ abbrev: q2
type: uimm2
doc: |
Quarter-word select for `<wrs2>`.
@@ -175,6 +181,7 @@
- `3`: Select `wrs1[WLEN-1:WLEN/4*3]` (most significant quarter-word)
- &mulqacc-acc-shift-imm
name: acc_shift_imm
+ abbrev: shift
type: uimm2<<6
doc: |
The number of bits to shift the `WLEN/2`-bit multiply result before accumulating.
@@ -264,6 +271,7 @@
- *mulqacc-zero-acc
- *mulqacc-wrd
- name: wrd_hwsel
+ abbrev: dh
type: enum(L,U)
doc: |
Half-word select for `<wrd>`.
@@ -698,16 +706,19 @@
Name of the GPR containing the memory byte address.
The value contained in the referenced GPR must be WLEN-aligned.
- name: offset
+ abbrev: "off"
doc: |
Offset value.
Must be WLEN-aligned.
type: simm<<5
- name: grs1_inc
+ abbrev: inc1
type: option(++)
doc: |
Increment the value in `<grs1>` by WLEN/8 (one word).
Cannot be specified together with `grd_inc`.
- name: grd_inc
+ abbrev: incd
type: option(++)
doc: |
Increment the value in `<grd>` by one.
@@ -768,17 +779,20 @@
- name: grs2
doc: Name of the GPR referencing the source WDR.
- name: offset
+ abbrev: "off"
doc: |
Offset value.
Must be WLEN-aligned.
type: simm<<5
- name: grs1_inc
type: option(++)
+ abbrev: inc1
doc: |
Increment the value in `<grs1>` by WLEN/8 (one word).
Cannot be specified together with `grs2_inc`.
- name: grs2_inc
type: option(++)
+ abbrev: inc2
doc: |
Increment the value in `<grs2>` by one.
Cannot be specified together with `grs1_inc`.
diff --git a/hw/ip/otbn/data/insns.yml b/hw/ip/otbn/data/insns.yml
index c0ed391..8fff402 100644
--- a/hw/ip/otbn/data/insns.yml
+++ b/hw/ip/otbn/data/insns.yml
@@ -92,6 +92,10 @@
#
# name: The name of the operand. Required and must be unique.
#
+# abbrev: An optional shortened name for the operand. If given, it
+# must not match the short name or full name of any other
+# operand.
+#
# type: The type of the operand. A string. If this can be figured out
# from the operand name, it is optional. See below for a list of
# possible operand types and the rules for recognising them
diff --git a/hw/ip/otbn/util/shared/insn_yaml.py b/hw/ip/otbn/util/shared/insn_yaml.py
index 7029a45..d9ebaee 100644
--- a/hw/ip/otbn/util/shared/insn_yaml.py
+++ b/hw/ip/otbn/util/shared/insn_yaml.py
@@ -53,6 +53,19 @@
self.operands,
lambda op: op.name)
+ # The call to index_list has checked that operand names are distinct.
+ # We also need to check that no operand abbreviation clashes with
+ # anything else.
+ operand_names = set(self.name_to_operand.keys())
+ for op in self.operands:
+ if op.abbrev is not None:
+ if op.abbrev in operand_names:
+ raise ValueError('The name {!r} appears as an operand or '
+ 'abbreviation more than once for '
+ 'instruction {!r}.'
+ .format(op.abbrev, self.mnemonic))
+ operand_names.add(op.abbrev)
+
if self.encoding is not None:
# If we have an encoding, we passed it to the Operand constructors
# above. This ensured that each operand has a field. However, it's
diff --git a/hw/ip/otbn/util/shared/operand.py b/hw/ip/otbn/util/shared/operand.py
index 1b0c0be..033bf96 100644
--- a/hw/ip/otbn/util/shared/operand.py
+++ b/hw/ip/otbn/util/shared/operand.py
@@ -757,16 +757,22 @@
# dict.
what = 'operand for {!r} instruction'.format(mnemonic)
if isinstance(yml, str):
- name = yml
+ name = yml.lower()
+ abbrev = None
op_type = None
doc = None
pc_rel = False
op_what = '{!r} {}'.format(name, what)
elif isinstance(yml, dict):
- yd = check_keys(yml, what, ['name'], ['type', 'pc-rel', 'doc'])
- name = check_str(yd['name'], 'name of ' + what)
+ yd = check_keys(yml, what,
+ ['name'],
+ ['type', 'pc-rel', 'doc', 'abbrev'])
+ name = check_str(yd['name'], 'name of ' + what).lower()
op_what = '{!r} {}'.format(name, what)
+ abbrev = get_optional_str(yd, 'abbrev', op_what)
+ if abbrev is not None:
+ abbrev = abbrev.lower()
op_type = get_optional_str(yd, 'type', op_what)
pc_rel = check_bool(yd.get('pc-rel', False),
'pc-rel field of ' + op_what)
@@ -784,7 +790,14 @@
.format(mnemonic, name))
enc_scheme_field = insn_encoding.fields[field_name].scheme_field
+ if abbrev is not None:
+ if name == abbrev:
+ raise ValueError('Operand {!r} of the {!r} instruction has '
+ 'an abbreviated name the same as its '
+ 'actual name.'
+ .format(name, mnemonic))
self.name = name
+ self.abbrev = abbrev
self.op_type = make_operand_type(op_type, pc_rel, name,
mnemonic, enc_scheme_field)
self.doc = doc
diff --git a/hw/ip/otbn/util/yaml_to_doc.py b/hw/ip/otbn/util/yaml_to_doc.py
index 3dbd2a1..8e698f3 100755
--- a/hw/ip/otbn/util/yaml_to_doc.py
+++ b/hw/ip/otbn/util/yaml_to_doc.py
@@ -156,14 +156,16 @@
return ''.join(parts)
-def name_op_enc_fields(encoding: Encoding) -> _O2EDicts:
+def name_op_enc_fields(name_to_operand: Dict[str, Operand],
+ encoding: Encoding) -> _O2EDicts:
'''Name the encoding fields corresponding to operators
In the generated documentation, we name encoding fields based on the
operand that the encode. For example, if the operand "foo" is encoded in a
field, the field will be labelled "FOO" in the table. If the field is split
over multiple bit ranges, they will be labelled like "FOO_0", "FOO_1" etc,
- counting from the LSB.
+ counting from the LSB. If an operand has an abbreviated name, this will be
+ used for the field instead of the full operand name.
Returns a pair of dicts: (o2e, e2o). o2e maps an operand name to the list
of (our names for) encoding fields that contribute to it, MSB first. e2o
@@ -190,6 +192,14 @@
# An encoding should never use an operand more than once
assert operand_name not in o2e
+ # Get the base name to use for fields. This is either an upper-case
+ # version of the operand name, or uses the operand's abbreviated name
+ # if available.
+ operand = name_to_operand.get(operand_name)
+ assert operand is not None
+ basename = operand_name if operand.abbrev is None else operand.abbrev
+ basename = basename.upper()
+
# There should always be at least one bit range for the field
scheme_field = field.scheme_field
assert scheme_field.bits.ranges
@@ -199,7 +209,7 @@
if len(scheme_field.bits.ranges) == 1:
msb = scheme_field.bits.ranges[0][0]
assert msb not in e2o
- range_name = operand_name.upper()
+ range_name = basename
o2e[operand_name] = [range_name]
e2o[msb] = range_name
continue
@@ -207,9 +217,8 @@
# Otherwise, we need to label the operands. Sorting ranges ensures that
# they appear LSB-first.
o2e_list = []
- range_basename = operand_name.upper()
for idx, (msb, lsb) in enumerate(sorted(scheme_field.bits.ranges)):
- range_name = '{}_{}'.format(range_basename, idx)
+ range_name = '{}_{}'.format(basename, idx)
o2e_list.append(range_name)
assert msb not in e2o
e2o[msb] = range_name
@@ -282,7 +291,7 @@
o2e = None
e2o = None
else:
- o2e, e2o = name_op_enc_fields(insn.encoding)
+ o2e, e2o = name_op_enc_fields(insn.name_to_operand, insn.encoding)
# Show the operand table if there is at least one operand and this isn't a
# pseudo-op.
