[otbn] Use libcst to extract implementations for docs
This isn't the full solution: we are going to want to do some
prettifying to make this more "pseudo-codeish". For example, we
currently have lines like
val1 = state.gprs.get_reg(self.grs1).read_unsigned()
state.gprs.get_reg(self.grd).write_unsigned(result)
which should probably be something more like
val1 <- gprs[self.grs1]
gprs[self.grd] <- val1
or similar. However, it does get rid of the duplicated definitions
from the YAML files and, if ugly, the definitions in the spec are now
at least clear.
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 2d8112d..60767bb 100644
--- a/hw/ip/otbn/data/base-insns.yml
+++ b/hw/ip/otbn/data/base-insns.yml
@@ -383,10 +383,6 @@
lsu:
type: csr
target: [csr]
- operation: |
- gpr_s_val = GPR[grs1]
- GPR[grd] = CSR[csr]
- CSR[csr] |= gpr_s_val
- mnemonic: csrrw
rv32i: true
@@ -408,13 +404,6 @@
lsu:
type: csr
target: [csr]
- operation: |
- gpr_s_val = GPR[grs1]
-
- if d != 0:
- GPR[grd] = CSR[csr]
-
- CSR[csr] = gpr_s_val
- mnemonic: ecall
rv32i: true
diff --git a/hw/ip/otbn/data/bignum-insns.yml b/hw/ip/otbn/data/bignum-insns.yml
index 6ab256c..5c3e72a 100644
--- a/hw/ip/otbn/data/bignum-insns.yml
+++ b/hw/ip/otbn/data/bignum-insns.yml
@@ -37,12 +37,6 @@
Adds two WDR values, writes the result to the destination WDR and updates
flags. The content of the second source WDR can be shifted by an unsigned
immediate before it is consumed by the operation.
- operation: |
- b_shifted = ShiftReg(wrs2, shift_type, shift_bits)
- (result, flags_out) = AddWithCarry(wrs1, b_shifted, "0")
-
- WDR[wrd] = result
- FLAGS[flag_group] = flags_out
encoding:
scheme: bnaf
mapping:
@@ -63,12 +57,6 @@
destination WDR, and updates the flags. The content of the second source
WDR can be shifted by an unsigned immediate before it is consumed by the
operation.
- operation: |
- b_shifted = ShiftReg(wrs2, shift_type, shift_bits)
- (result, flags_out) = AddWithCarry(wrs1, b_shifted, FLAGS[flag_group].C)
-
- WDR[wrd] = result
- FLAGS[flag_group] = flags_out
encoding:
scheme: bnaf
mapping:
@@ -96,11 +84,6 @@
doc: |
Adds a zero-extended unsigned immediate to the value of a WDR, writes the
result to the destination WDR, and updates the flags.
- operation: |
- (result, flags_out) = AddWithCarry(wrs1, imm, "0")
-
- WDR[wrd] = result
- FLAGS[flag_group] = flags_out
encoding:
scheme: bnai
mapping:
@@ -125,13 +108,6 @@
The intermediate result is small enough if both inputs are less than `MOD`.
Flags are not used or saved.
- operation: |
- result = wrs1 + wrs2
-
- if result >= MOD:
- result = result - MOD
-
- WDR[wrd] = result & ((1 << 256) - 1)
encoding:
scheme: bnam
mapping:
@@ -192,16 +168,6 @@
Multiplies two `WLEN/4` WDR values, shifts the product by `acc_shift_imm` bits, and adds the result to the accumulator.
For versions of the instruction with writeback, see `BN.MULQACC.WO` and `BN.MULQACC.SO`.
- operation: |
- a_qw = GetQuarterWord(wrs1, wrs1_qwsel)
- b_qw = GetQuarterWord(wrs2, wrs2_qwsel)
-
- mul_res = a_qw * b_qw
-
- if zero_acc:
- ACC = 0
-
- ACC = ACC + (mul_res << acc_shift_imm)
encoding:
scheme: bnaq
mapping:
@@ -235,22 +201,6 @@
doc: |
Multiplies two `WLEN/4` WDR values, shifts the product by `acc_shift_imm` bits, and adds the result to the accumulator.
Writes the resulting accumulator to `wrd`.
- Updates the M, L and Z flags of `flag_group`.
- operation: |
- a_qw = GetQuarterWord(wrs1, wrs1_qwsel)
- b_qw = GetQuarterWord(wrs2, wrs2_qwsel)
-
- mul_res = a_qw * b_qw
-
- if zero_acc:
- ACC = 0
-
- ACC = ACC + (mul_res << acc_shift_imm)
-
- WDR[wrd] = ACC
- FLAGS[flag_group].M = ACC[WLEN-1]
- FLAGS[flag_group].L = ACC[0]
- FLAGS[flag_group].Z = (ACC == 0)
encoding:
scheme: bnaq
mapping:
@@ -299,29 +249,6 @@
If `wrd_hwsel` is one (so the instruction is updating the upper half-word of `wrd`), it updates the `M` and `Z` flags and leaves `L` unchanged.
The `M` flag is set iff the top bit of the shifted-out result is zero.
The `Z` flag is left unchanged if the shifted-out result is zero and cleared if not.
- operation: |
- a_qw = GetQuarterWord(wrs1, wrs1_qwsel)
- b_qw = GetQuarterWord(wrs2, wrs2_qwsel)
-
- mul_res = a_qw * b_qw
-
- if zero_acc:
- ACC = 0
-
- ACC = ACC + (mul_res << acc_shift_imm)
-
- shifted = ACC[WLEN/2-1:0]
- ACC = ACC >> (WLEN/2)
-
- if wrd_hwsel == 'L':
- WDR[wrd][WLEN/2-1:0] = shifted
- FLAGS[flag_group].L = shifted[0]
- FLAGS[flag_group].Z = (shifted == 0)
- elif wrd_hwsel == 'U':
- WDR[wrd][WLEN-1:WLEN/2] = shifted
- FLAGS[flag_group].M = shifted[WLEN/2-1]
- if (shifted != 0):
- FLAGS[flag_group].Z = 0
encoding:
scheme: bnaq
mapping:
@@ -352,12 +279,6 @@
doc: |
Subtracts the second WDR value from the first one, writes the result to the destination WDR and updates flags.
The content of the second source WDR can be shifted by an unsigned immediate before it is consumed by the operation.
- operation: |
- b_shifted = ShiftReg(wrs2, shift_type, shift_bits)
- (result, flags_out) = SubtractWithBorrow(wrs1, b_shifted, 0)
-
- WDR[wrd] = result
- FLAGS[flag_group] = flags_out
encoding:
scheme: bnaf
mapping:
@@ -376,12 +297,6 @@
doc: |
Subtracts the second WDR value and the Carry from the first one, writes the result to the destination WDR, and updates the flags.
The content of the second source WDR can be shifted by an unsigned immediate before it is consumed by the operation.
- operation: |
- b_shifted = ShiftReg(wrs2, shift_type, shift_bits)
- (result, flags_out) = SubtractWithBorrow(wrs1, b_shifted, FLAGS[flag_group].C)
-
- WDR[wrd] = result
- FLAGS[flag_group] = flags_out
encoding:
scheme: bnaf
mapping:
@@ -408,11 +323,6 @@
doc: |
Subtracts a zero-extended unsigned immediate from the value of a WDR,
writes the result to the destination WDR, and updates the flags.
- operation: |
- (result, flags_out) = SubtractWithBorrow(wrs1, imm, 0)
-
- WDR[wrd] = result
- FLAGS[flag_group] = flags_out
encoding:
scheme: bnai
mapping:
@@ -437,13 +347,6 @@
This is guaranteed if both inputs are less than `MOD`.
Flags are not used or saved.
- operation: |
- result = wrs1 - wrs2
-
- if result < 0:
- result = MOD + result
-
- WDR[wrd] = result & ((1 << 256) - 1)
encoding:
scheme: bnam
mapping:
@@ -472,13 +375,6 @@
Takes the values stored in registers referenced by `wrs1` and `wrs2` and stores the result in the register referenced by `wrd`.
The content of the second source register can be shifted by an immediate before it is consumed by the operation.
The M, L and Z flags in flag group 0 are updated with the result of the operation.
- operation: |
- b_shifted = ShiftReg(wrs2, shift_type, shift_bits)
- result = wrs1 & b_shifted
-
- WDR[wrd] = result
- flags_out = FlagsForResult(result)
- FLAGS[flag_group] = {M: flags_out.M, L: flags_out.L, Z: flags_out.Z, C: FLAGS[flag_group].C}
encoding:
scheme: bna
mapping:
@@ -499,13 +395,6 @@
Takes the values stored in WDRs referenced by `wrs1` and `wrs2` and stores the result in the WDR referenced by `wrd`.
The content of the second source WDR can be shifted by an immediate before it is consumed by the operation.
The M, L and Z flags in flag group 0 are updated with the result of the operation.
- operation: |
- b_shifted = ShiftReg(wrs2, shift_type, shift_bits)
- result = wrs1 | b_shifted
-
- WDR[wrd] = result
- flags_out = FlagsForResult(result)
- FLAGS[flag_group] = {M: flags_out.M, L: flags_out.L, Z: flags_out.Z, C: FLAGS[flag_group].C}
encoding:
scheme: bna
mapping:
@@ -533,13 +422,6 @@
Negates the value in `wrs` and stores the result in the register referenced by `wrd`.
The source value can be shifted by an immediate before it is consumed by the operation.
The M, L and Z flags in flag group 0 are updated with the result of the operation.
- operation: |
- a_shifted = ShiftReg(wrs1, shift_type, shift_bits)
- result = ~a_shifted
-
- WDR[wrd] = result
- flags_out = FlagsForResult(result)
- FLAGS[flag_group] = {M: flags_out.M, L: flags_out.L, Z: flags_out.Z, C: FLAGS[flag_group].C}
encoding:
scheme: bnan
mapping:
@@ -559,13 +441,6 @@
Takes the values stored in WDRs referenced by `wrs1` and `wrs2` and stores the result in the WDR referenced by `wrd`.
The content of the second source WDR can be shifted by an immediate before it is consumed by the operation.
The M, L and Z flags in flag group 0 are updated with the result of the operation.
- operation: |
- b_shifted = ShiftReg(wrs2, shift_type, shift_bits)
- result = wrs1 ^ b_shifted
-
- WDR[wrd] = result
- flags_out = FlagsForResult(result)
- FLAGS[flag_group] = {M: flags_out.M, L: flags_out.L, Z: flags_out.Z, C: FLAGS[flag_group].C}
encoding:
scheme: bna
mapping:
@@ -595,8 +470,6 @@
doc: |
Concatenates the content of WDRs referenced by `wrs1` and `wrs2` (`wrs1` forms the upper part), shifts it right by an immediate value and truncates to WLEN bit.
The result is stored in the WDR referenced by `wrd`.
- operation: |
- WDR[wrd] = (((wrs1 << WLEN) | wrs2) >> imm)[WLEN-1:0]
encoding:
scheme: bnr
mapping:
@@ -628,10 +501,6 @@
<wrd>, <wrs1>, <wrs2>, [FG<flag_group>.]<flag>
doc: |
Returns in the destination WDR the value of the first source WDR if the flag in the chosen flag group is set, otherwise returns the value of the second source WDR.
- operation: |
- flag_is_set = FLAGS[flag_group].get(flag)
-
- WDR[wrd] = wrs1 if flag_is_set else wrs2
encoding:
scheme: bns
mapping:
@@ -656,11 +525,6 @@
doc: |
Subtracts the second WDR value from the first one and updates flags.
This instruction is identical to BN.SUB, except that no result register is written.
- operation: |
- b_shifted = ShiftReg(wrs2, shift_type, shift_bits)
- (, flags_out) = SubtractWithBorrow(wrs1, b_shifted, 0)
-
- FLAGS[flag_group] = flags_out
encoding:
scheme: bnc
mapping:
@@ -678,10 +542,6 @@
doc: |
Subtracts the second WDR value from the first one and updates flags.
This instruction is identical to BN.SUBB, except that no result register is written.
- operation: |
- (, flags_out) = SubtractWithBorrow(wrs1, wrs2, FLAGS[flag_group].C)
-
- FLAGS[flag_group] = flags_out
encoding:
scheme: bnc
mapping:
@@ -740,24 +600,6 @@
The memory address must be aligned to WLEN bits.
Any address that is unaligned or is above the top of memory results in an error (setting bit `bad_data_addr` in `ERR_BITS`).
cycles: 2
- operation: |
- mem_addr = GPR[grs1] + offset
- wdr_dest = GPR[grd]
-
- if grs1_inc and grd_inc:
- raise IllegalInsn()
-
- if mem_addr % (WLEN / 8) or mem_addr + WLEN > DMEM_SIZE:
- raise BadDataAddr()
-
- mem_index = mem_addr // (WLEN / 8)
-
- WDR[wdr_dest] = LoadWlenWordFromMemory(mem_index)
-
- if grs1_inc:
- GPR[grs1] = GPR[grs1] + (WLEN / 8)
- if grd_inc:
- GPR[grd] = (GPR[grd] + 1) & 0x1f
lsu:
type: mem-load
target: [offset, grs1]
@@ -815,24 +657,6 @@
The memory address must be aligned to WLEN bits.
Any address that is unaligned or is above the top of memory results in an error (setting bit `bad_data_addr` in `ERR_BITS`).
- operation: |
- mem_addr = GPR[grs1] + offset
- wdr_src = GPR[grs2]
-
- if grs1_inc and grs2_inc:
- raise IllegalInsn()
-
- if mem_addr % (WLEN / 8) or mem_addr + WLEN > DMEM_SIZE:
- raise BadDataAddr()
-
- mem_index = mem_addr // (WLEN / 8)
-
- StoreWlenWordToMemory(mem_index, WDR[wdr_src])
-
- if grs1_inc:
- GPR[grs1] = GPR[grs1] + (WLEN / 8)
- if grs2_inc:
- GPR[grs2] = (GPR[grs2] + 1) & 0x1f
lsu:
type: mem-store
target: [offset, grs1]
@@ -850,7 +674,6 @@
- mnemonic: bn.mov
synopsis: Copy content between WDRs (direct addressing)
operands: [wrd, wrs]
- operation: WDR[wrd] = WDR[wrs]
encoding:
scheme: bnmov
mapping:
@@ -888,16 +711,6 @@
- If `grd_inc` is set, `grd` is updated to be `(*grd + 1) & 0x1f`.
- If `grs_inc` is set, `grs` is updated to be `(*grs + 1) & 0x1f`.
- operation: |
- WDR[GPR[grd]] = WDR[GPR[grs]]
-
- if grd_inc and grs_inc:
- raise IllegalInsn()
-
- if grs_inc:
- GPR[grs] = GPR[grs] + 1
- if grd_inc:
- GPR[grd] = GPR[grd] + 1
encoding:
scheme: bnmovr
mapping:
@@ -916,8 +729,6 @@
doc: |
Reads a WSR to a WDR.
If `wsr` isn't the index of a valid WSR, this halts with an error (TODO: Specify error code).
- operation: |
- WDR[wrd] = WSR[wsr]
encoding:
scheme: wcsr
mapping:
@@ -940,8 +751,6 @@
doc: |
Writes a WDR to a WSR.
If `wsr` isn't the index of a valid WSR, this halts with an error (TODO: Specify error code).
- operation: |
- WSR[wsr] = WDR[wrs]
encoding:
scheme: wcsr
mapping:
diff --git a/hw/ip/otbn/util/Makefile b/hw/ip/otbn/util/Makefile
index 863b9e2..f2024a9 100644
--- a/hw/ip/otbn/util/Makefile
+++ b/hw/ip/otbn/util/Makefile
@@ -15,7 +15,7 @@
$(build-dir) $(lint-build-dir):
mkdir -p $@
-pylibs := $(wildcard shared/*.py)
+pylibs := $(wildcard shared/*.py docs/*.py)
pyscripts := yaml_to_doc.py otbn-as otbn-ld otbn-objdump
lint-stamps := $(foreach s,$(pyscripts),$(lint-build-dir)/$(s).stamp)
diff --git a/hw/ip/otbn/util/docs/__init__.py b/hw/ip/otbn/util/docs/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/hw/ip/otbn/util/docs/__init__.py
diff --git a/hw/ip/otbn/util/docs/get_impl.py b/hw/ip/otbn/util/docs/get_impl.py
new file mode 100644
index 0000000..48ec5ec
--- /dev/null
+++ b/hw/ip/otbn/util/docs/get_impl.py
@@ -0,0 +1,53 @@
+# Copyright lowRISC contributors.
+# Licensed under the Apache License, Version 2.0, see LICENSE for details.
+# SPDX-License-Identifier: Apache-2.0
+
+from typing import Dict, Optional, Sequence
+
+import libcst as cst
+
+
+class ImplVisitor(cst.CSTVisitor):
+ '''An AST visitor used to extract documentation from the ISS'''
+ def __init__(self) -> None:
+ self.impls = {} # type: Dict[str, Sequence[cst.BaseStatement]]
+
+ self.cur_class = None # type: Optional[str]
+
+ def visit_ClassDef(self, node: cst.ClassDef) -> None:
+ assert self.cur_class is None
+ self.cur_class = node.name.value
+
+ def leave_ClassDef(self, node: cst.ClassDef) -> None:
+ self.cur_class = None
+
+ def leave_FunctionDef(self, node: cst.FunctionDef) -> None:
+ if ((self.cur_class is None or
+ node.name.value != 'execute' or
+ self.cur_class in self.impls)):
+ return
+
+ # The body of a function definition is always an IndentedBlock. Strip
+ # that out to get at the statements inside.
+ assert isinstance(node.body, cst.IndentedBlock)
+ self.impls[self.cur_class] = node.body.body
+
+
+def read_implementation(path: str) -> Dict[str, str]:
+ '''Read the implementation at path (probably insn.py)
+
+ Returns a dictionary from instruction class name to its pseudo-code
+ implementation. An instruction class name looks like ADDI (for addi) or
+ BNADDM (for bn.addm).
+
+ '''
+ with open(path, 'r') as handle:
+ node = cst.parse_module(handle.read())
+
+ # Extract the function bodies
+ visitor = ImplVisitor()
+ node.visit(visitor)
+
+ # Render the function bodies
+ return {cls: ''.join(node.code_for_node(stmt) for stmt in body)
+ for cls, body in visitor.impls.items()}
diff --git a/hw/ip/otbn/util/shared/insn_yaml.py b/hw/ip/otbn/util/shared/insn_yaml.py
index 3234899..906905b 100644
--- a/hw/ip/otbn/util/shared/insn_yaml.py
+++ b/hw/ip/otbn/util/shared/insn_yaml.py
@@ -27,7 +27,7 @@
['mnemonic', 'operands'],
['group', 'rv32i', 'synopsis',
'syntax', 'doc', 'note', 'trailing-doc',
- 'operation', 'encoding', 'glued-ops',
+ 'encoding', 'glued-ops',
'literal-pseudo-op', 'python-pseudo-op', 'lsu',
'straight-line', 'cycles'])
@@ -86,7 +86,6 @@
self.doc = get_optional_str(yd, 'doc', what)
self.note = get_optional_str(yd, 'note', what)
self.trailing_doc = get_optional_str(yd, 'trailing-doc', what)
- self.operation = get_optional_str(yd, 'operation', what)
raw_syntax = get_optional_str(yd, 'syntax', what)
if raw_syntax is not None:
diff --git a/hw/ip/otbn/util/yaml_to_doc.py b/hw/ip/otbn/util/yaml_to_doc.py
index 8e698f3..f4bf697 100755
--- a/hw/ip/otbn/util/yaml_to_doc.py
+++ b/hw/ip/otbn/util/yaml_to_doc.py
@@ -15,6 +15,8 @@
from shared.insn_yaml import Insn, InsnsFile, InsnGroup, load_file
from shared.operand import EnumOperandType, OptionOperandType, Operand
+from docs.get_impl import read_implementation
+
_O2EDicts = Tuple[Dict[str, List[str]], Dict[int, str]]
@@ -229,7 +231,7 @@
return (o2e, e2o)
-def render_insn(insn: Insn, heading_level: int) -> str:
+def render_insn(insn: Insn, impl: Optional[str], heading_level: int) -> str:
'''Generate the documentation for an instruction
heading_level is the current Markdown heading level. It should be greater
@@ -307,8 +309,7 @@
if insn.literal_pseudo_op is not None:
parts.append(render_literal_pseudo_op(insn.literal_pseudo_op))
- # Show operation pseudo-code if given
- if insn.operation is not None:
+ if impl is not None:
parts.append('{} Operation\n\n'
.format('#' * (heading_level + 1)))
@@ -335,14 +336,18 @@
'to assembly syntax.\n\n'
.format(op_str))
- parts.append('```python3\n'
- '{}\n'
+ # Note: No trailing newline after the inserted contents because libcst
+ # (which we use for extracting documentation) always adds a trailing
+ # newline itself.
+ parts.append('```\n'
+ '{}'
'```\n\n'
- .format(insn.operation))
+ .format(impl))
return ''.join(parts)
def render_insn_group(group: InsnGroup,
+ impls: Dict[str, str],
heading_level: int,
out_file: TextIO) -> None:
# We don't print the group heading: that's done in the top-level
@@ -355,28 +360,33 @@
return
for insn in group.insns:
- out_file.write(render_insn(insn, heading_level))
+ class_name = insn.mnemonic.replace('.', '').upper()
+ impl = impls.get(class_name)
+ out_file.write(render_insn(insn, impl, heading_level))
def render_insns(insns: InsnsFile,
+ impls: Dict[str, str],
heading_level: int,
out_dir: str) -> None:
'''Render documentation for all instructions'''
for group in insns.groups.groups:
group_path = os.path.join(out_dir, group.key + '.md')
with open(group_path, 'w') as group_file:
- render_insn_group(group, heading_level, group_file)
+ render_insn_group(group, impls, heading_level, group_file)
def main() -> int:
parser = argparse.ArgumentParser()
parser.add_argument('yaml_file')
+ parser.add_argument('py_file')
parser.add_argument('out_dir')
args = parser.parse_args()
try:
insns = load_file(args.yaml_file)
+ impls = read_implementation(args.py_file)
except RuntimeError as err:
print(err, file=sys.stderr)
return 1
@@ -387,7 +397,7 @@
print('Failed to create output directory {!r}: {}.'
.format(args.out_dir, err))
- render_insns(insns, 2, args.out_dir)
+ render_insns(insns, impls, 2, args.out_dir)
return 0
diff --git a/python-requirements.txt b/python-requirements.txt
index 34ebc0a..bc803e2 100644
--- a/python-requirements.txt
+++ b/python-requirements.txt
@@ -8,6 +8,7 @@
hjson
ipyxact >= 0.2.4
isort
+libcst
livereload
mako
meson >= 0.53.0, <= 0.54 # minimum matches version in meson.build
diff --git a/util/build_docs.py b/util/build_docs.py
index 477361f..4aa6e81 100755
--- a/util/build_docs.py
+++ b/util/build_docs.py
@@ -330,9 +330,11 @@
otbn_dir = SRCTREE_TOP / 'hw/ip/otbn'
script = otbn_dir / 'util/yaml_to_doc.py'
yaml_file = otbn_dir / 'data/insns.yml'
+ impl_file = otbn_dir / 'dv/otbnsim/sim/insn.py'
out_dir = config['outdir-generated'].joinpath('otbn-isa')
- subprocess.run([str(script), str(yaml_file), str(out_dir)], check=True)
+ subprocess.run([str(script), str(yaml_file), str(impl_file), str(out_dir)],
+ check=True)
def hugo_match_version(hugo_bin_path, version):
