From 58cae892d285c5dc85fb27456a584d6144e02196 Mon Sep 17 00:00:00 2001 From: hanhanW Date: Fri, 21 Aug 2026 09:32:59 -0700 Subject: [PATCH 1/5] [rocjitsu] Add validated ISA additions Public MR ISA snapshots may omit target-variant instructions and the encoding identifiers needed to decode them. Editing the checked-in base XML would obscure its provenance and make future snapshot updates harder to audit, while adding instructions alone can leave their opcodes unreachable from the decode table. Add a narrow, ordered ISA additions format for complete Instruction nodes and complete identifiers belonging to existing encodings. Validate base metadata, references, opcode ownership, identifier layout, collisions, and implied-literal relationships before merging the additions. Merge validated identifiers before normal encoding parsing so added instructions follow the existing decode-table path. Reject active instruction forms that remain unreachable, and retain additions provenance on IsaSpec and Instruction for later target-variant handling. Wire repeatable --isa-additions NAME:XML arguments into single- and multi-ISA generation. Keep the public MR ISA XML unchanged and preserve byte-identical generated output for empty additions documents. This change adds generator infrastructure only. It does not add gfx1251 instructions or modify generated runtime sources. Tests cover validation failures, ordered and atomic application, exact CDNA5 decode-table mapping, generated decoder dispatch, and byte-identical generation for empty additions documents. Signed-off-by: hanhanW --- emulation/rocjitsu/docs/codegen.md | 76 +- .../rocjitsu/lib/python/amdisa/__init__.py | 5 + .../rocjitsu/lib/python/amdisa/__main__.py | 53 +- .../rocjitsu/lib/python/amdisa/gpuisa.py | 15 + .../lib/python/amdisa/isa_additions.py | 838 +++++++++++++++ .../rocjitsu/lib/python/amdisa/parser.py | 109 +- .../python/amdisa/tests/test_isa_additions.py | 983 ++++++++++++++++++ 7 files changed, 2030 insertions(+), 49 deletions(-) create mode 100644 emulation/rocjitsu/lib/python/amdisa/isa_additions.py create mode 100644 emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py diff --git a/emulation/rocjitsu/docs/codegen.md b/emulation/rocjitsu/docs/codegen.md index df561288aa68..28bf8eaa2718 100644 --- a/emulation/rocjitsu/docs/codegen.md +++ b/emulation/rocjitsu/docs/codegen.md @@ -39,6 +39,79 @@ while its installed dependencies come from the environment. rocm-systems/shared/machine-readable-isa/isa/ ``` +## ISA additions + +Repository-owned ISA additions can be supplied without modifying the public MR +ISA XML. Each ISA additions document has this shape: + +```xml + + + + ENC_EXAMPLE + 7 + ...complete identifier... + + + + + + + + +``` + +Apply one or more additions documents with a logical ISA name. Repeated +documents for one ISA are processed in command-line and document order: + +```bash +python -m amdisa \ + cdna5:/path/to/amdgpu_isa_cdna5.xml \ + --isa-additions cdna5:/path/to/first-additions.xml \ + --isa-additions cdna5:/path/to/second-additions.xml \ + --isa-output /path/to/isa-output \ + --dbt-output /path/to/dbt-output +``` + +The `EncodingIdentifierAdditions` element is optional. `InstructionAdditions` +is required and may be empty. The additions contract is intentionally narrower +than a general XML patch: + +- Only complete `` nodes and complete identifiers for an existing + encoding's `` list are accepted. An additions document + cannot replace or delete base definitions, identifier masks, microcode + formats, encoding conditions, operand types, or data formats. +- Added instructions must reference encodings and operand types already + defined by the base XML. +- An identifier addition names its encoding and expected opcode. Its radix and + width must match the base encoding; its fixed bits must match an existing + identifier layout; and its decoded opcode must equal the declared opcode. + Duplicate and colliding decode slots are rejected. Each identifier must be + owned by an added instruction. An implied-literal identifier must have the + same instruction owner and opcode in its parent encoding. +- Additions document IDs and instruction names must be unique. Encoding/opcode + ownership collisions between different instructions, repeated instruction + forms, out-of-range opcodes, missing references, unknown root data, and base + architecture/schema mismatches are errors. Repetition of one instruction's + slot under distinct MR-ISA encoding conditions is allowed. +- Every input document and contained addition validates before the parser-owned + tree is changed. Validated identifiers are merged before normal + encoding/decode-table parsing, then instructions are appended in deterministic + file/document order. A bad later file cannot leave a partially merged + specification, and an active added instruction whose final opcode pointer + would remain unpopulated is an error. +- An empty additions document leaves generated output byte-identical to the base + XML. + +The parsed `IsaSpec.applied_additions` and each added +`Instruction.source_addition` retain the source document's provenance for later +generator stages. +Provenance does not by itself define target availability: a separate variant +model must also be able to restrict encoding forms already present in the base +XML. + ## Generated file locations | Generated files | Location | Generator | @@ -69,7 +142,7 @@ decode/encode functions and neutral field structs are auto-generated. ## CLI reference ``` -python -m amdisa [--gen-isas] [--gen-dbt] +python -m amdisa [--isa-additions NAME:XML] [--gen-isas] [--gen-dbt] [--isa-output DIR] [--include-root DIR] [--dbt-output DIR] [NAME:]XML ... ``` @@ -77,6 +150,7 @@ python -m amdisa [--gen-isas] [--gen-dbt] | Option | Description | |---|---| | `[NAME:]XML ...` | One or more ISA XMLs. A recognized name selects its semantic profile. An unrecognized explicit name is treated as a custom generated identity, with its semantic profile detected from the XML | +| `--isa-additions NAME:XML` | Apply a validated ISA additions file to the named ISA; may be repeated | | `--gen-isas` | Generate ISA C++ files (decoders, encodings, execute bodies) | | `--gen-dbt` | Generate DBT legalization tables and encoding translators | | `--isa-output DIR` | Output path for generated ISA C++ files | diff --git a/emulation/rocjitsu/lib/python/amdisa/__init__.py b/emulation/rocjitsu/lib/python/amdisa/__init__.py index 6e87305fcf4e..4b2285f03aab 100644 --- a/emulation/rocjitsu/lib/python/amdisa/__init__.py +++ b/emulation/rocjitsu/lib/python/amdisa/__init__.py @@ -7,9 +7,11 @@ from amdisa.gpuisa import ( InstEncoding, Instruction, + IsaAdditionProvenance, IsaSpec, Operand, ) +from amdisa.isa_additions import IsaAdditionError, apply_isa_additions from amdisa.isa_profile import ( Cdna1Profile, Cdna2Profile, @@ -52,6 +54,8 @@ 'Cdna5Profile', 'InstEncoding', 'Instruction', + 'IsaAdditionError', + 'IsaAdditionProvenance', 'IsaProfile', 'MemoryCoherencyModel', 'MnemonicRule', @@ -63,6 +67,7 @@ 'IsaSpec', 'Operand', 'Parser', + 'apply_isa_additions', 'SchemaValueError', 'SchemaVersion', 'InstructionSemantics', diff --git a/emulation/rocjitsu/lib/python/amdisa/__main__.py b/emulation/rocjitsu/lib/python/amdisa/__main__.py index 79886547e07f..a458b2258129 100644 --- a/emulation/rocjitsu/lib/python/amdisa/__main__.py +++ b/emulation/rocjitsu/lib/python/amdisa/__main__.py @@ -60,6 +60,23 @@ } +def _group_isa_additions_args(entries: list[str] | None) -> dict[str, list[str]]: + """Group repeated ``NAME:XML`` additions arguments by logical ISA name.""" + grouped: dict[str, list[str]] = {} + for entry in entries or []: + if ':' not in entry: + raise ValueError( + f'--isa-additions entry must be name:xml_path, got: {entry}' + ) + name, xml_path = entry.split(':', 1) + if not name or not xml_path: + raise ValueError( + f'--isa-additions entry must have non-empty name and path, got: {entry}' + ) + grouped.setdefault(name, []).append(xml_path) + return grouped + + def _collect_shared_execute_body_variants(specs, plan): """Collect candidate shared execute bodies from each ISA. @@ -193,11 +210,35 @@ def _codegen_config( def _run(args) -> None: """Parse the input XMLs and generate the requested outputs.""" - specs = [] + try: + addition_xmls = _group_isa_additions_args(getattr(args, 'isa_additions', None)) + except ValueError as error: + print(f'error: {error}', file=sys.stderr) + sys.exit(1) + + parsed_inputs = [] + logical_names = set() for entry in args.isafiles: name, xml_path, profile_key = _parse_isa_arg(entry) profile = _PROFILES[profile_key]() - spec = Parser(xml_path, profile).parse() + logical_name = ( + name or profile.generated_arch_name or profile_key.replace('.', '_') + ) + parsed_inputs.append((name, logical_name, xml_path, profile)) + logical_names.add(logical_name) + + unknown_addition_names = set(addition_xmls) - logical_names + if unknown_addition_names: + names = ', '.join(sorted(unknown_addition_names)) + print( + f'error: --isa-additions names not present in ISA inputs: {names}', + file=sys.stderr, + ) + sys.exit(1) + + specs = [] + for name, logical_name, xml_path, profile in parsed_inputs: + spec = Parser(xml_path, profile, addition_xmls.get(logical_name, ())).parse() _apply_codegen_identity(spec, name) sem = derive_all_semantics(spec) specs.append((spec.arch_name, spec, sem)) @@ -312,6 +353,14 @@ def main() -> None: help='Machine-readable AMD GPU ISA XML specifications. Each path may ' 'have a generated identity prefix (e.g., cdna1:/path/to/cdna1.xml).', ) + arg_parser.add_argument( + '--isa-additions', + action='append', + default=[], + metavar='NAME:XML', + help='apply an ISA additions XML file to the named ISA. May be repeated; ' + 'files are applied in command-line order.', + ) arg_parser.add_argument( '--gen-isas', action='store_true', diff --git a/emulation/rocjitsu/lib/python/amdisa/gpuisa.py b/emulation/rocjitsu/lib/python/amdisa/gpuisa.py index d8b6af252f07..9d3f56a1da7c 100644 --- a/emulation/rocjitsu/lib/python/amdisa/gpuisa.py +++ b/emulation/rocjitsu/lib/python/amdisa/gpuisa.py @@ -243,6 +243,8 @@ class Instruction(InstBase): classes. ``None`` means encoding provenance is unknown; modifier generation rejects that state, so synthetic VOP instructions must provide an explicit set. + source_addition: Additions provenance for a repository-supplied instruction, + or ``None`` when the instruction came from the base MR ISA XML. """ def __init__( @@ -253,12 +255,14 @@ def __init__( operands: list[Operand], is_implied_literal_enc: bool = False, available_encodings: frozenset[str] | None = None, + source_addition: IsaAdditionProvenance | None = None, ) -> None: super().__init__(enc_name, is_implied_literal_enc) self.name = name self.opcode = opcode self.operands = operands self.available_encodings = available_encodings + self.source_addition = source_addition @cached_property def fmt_name(self) -> str: @@ -332,6 +336,14 @@ class DecodeTableEntry: decode_func: str | None = None +@dataclass(frozen=True) +class IsaAdditionProvenance: + """Identity and source path for one applied ISA additions document.""" + + identifier: str + path: str + + class IsaSpec: """Internal representation of a machine-readable ISA spec. @@ -357,6 +369,8 @@ class IsaSpec: encoding conditions indicate an implied literal DWORD. Populated during parsing using the profile's ``is_implied_literal_encoding()`` method. + applied_additions: Ordered provenance for ISA additions documents + merged into this specification. """ def __init__( @@ -380,6 +394,7 @@ def __init__( 2, profile.max_enc_bits ) self.alt_encs_with_implied_literal: set[str] = set() + self.applied_additions: tuple[IsaAdditionProvenance, ...] = () # Every fieldless operand type observed while parsing this spec. # Consumed by fieldless_policy.validate_fieldless_taxonomy. self.fieldless_operand_types: set[str] = set() diff --git a/emulation/rocjitsu/lib/python/amdisa/isa_additions.py b/emulation/rocjitsu/lib/python/amdisa/isa_additions.py new file mode 100644 index 000000000000..50068a4e78b7 --- /dev/null +++ b/emulation/rocjitsu/lib/python/amdisa/isa_additions.py @@ -0,0 +1,838 @@ +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# SPDX-License-Identifier: MIT + +"""Validated instruction and encoding-identifier additions for MR ISA XMLs. + +The vendor MR ISA XML remains immutable. An additions document may add complete +```` nodes and complete identifiers to an existing encoding's +```` list. It cannot replace or delete existing data. +""" + +from __future__ import annotations + +from collections.abc import Mapping, Sequence +from copy import deepcopy +from dataclasses import dataclass +import re +import xml.etree.ElementTree as elem_tree + +from amdisa import xml_schema as xs +from amdisa.gpuisa import IsaAdditionProvenance +from amdisa.isa_profile import IsaProfile + +ADDITIONS_ROOT = 'IsaAdditions' +INSTRUCTION_ADDITIONS = 'InstructionAdditions' +IDENTIFIER_ADDITIONS = 'EncodingIdentifierAdditions' +IDENTIFIER_ADDITION = 'EncodingIdentifierAddition' +ADDITIONS_ID_ATTR = 'Id' +ADDITIONS_BASE_ARCH_ATTR = 'BaseArchitecture' +ADDITIONS_BASE_SCHEMA_ATTR = 'BaseSchemaVersion' +ADDITION_SOURCE_ATTR = '_amdisa_addition_id' + +_ADDITIONS_ATTRIBUTES = { + ADDITIONS_ID_ATTR, + ADDITIONS_BASE_ARCH_ATTR, + ADDITIONS_BASE_SCHEMA_ATTR, +} +_ID_PATTERN = re.compile(r'[A-Za-z0-9][A-Za-z0-9._-]*\Z') + + +class IsaAdditionError(ValueError): + """An ISA additions document is malformed or conflicts with its base XML.""" + + +@dataclass(frozen=True) +class _EncodingDefinition: + """Base-owned encoding information needed to validate identifier additions.""" + + name: str + identifiers_node: elem_tree.Element + bit_width: int + radix: int + flat_encoding_slice: tuple[int, int] + opcode_slice: tuple[int, int] + dont_care_bits: int + opcode_bit_count: int + opcode_modifier_bit_count: int + base_layout_signatures: frozenset[str] + base_encoding_values: frozenset[int] + identifier_texts: frozenset[str] + decoded_opcodes: frozenset[int] + parent_name: str | None + is_implied_literal: bool + + @property + def opcode_limit(self) -> int: + return 1 << (self.opcode_bit_count + self.opcode_modifier_bit_count) + + +@dataclass(frozen=True) +class _InstructionAddition: + node: elem_tree.Element + name: str + path: str + addition_id: str + forms: tuple[tuple[str, int, str], ...] + + +@dataclass(frozen=True) +class _IdentifierAddition: + node: elem_tree.Element + encoding: _EncodingDefinition + opcode: int + + +def parse_encoding_identifier_mask( + enc_id_mask: str, + max_enc_bits: int, + enc_field_bit_cnt: int, + op_field_bit_cnt: int, +) -> tuple[tuple[int, int], tuple[int, int], int]: + """Derive primary-encoding and opcode slices from an identifier mask.""" + bit_masks = [ + (match.start(), match.end()) for match in re.finditer(r'1+', enc_id_mask) + ] + if not bit_masks: + raise IsaAdditionError('encoding identifier mask contains no set bits') + flat_enc_mask = bit_masks[0] + if len(bit_masks) == 1: + op_mask = ( + bit_masks[0][0] + enc_field_bit_cnt, + bit_masks[0][0] + enc_field_bit_cnt + op_field_bit_cnt, + ) + if (flat_enc_mask[1] - flat_enc_mask[0]) > max_enc_bits: + flat_enc_mask = ( + flat_enc_mask[0], + flat_enc_mask[0] + max_enc_bits, + ) + else: + op_mask = ( + bit_masks[1][0], + bit_masks[1][0] + op_field_bit_cnt, + ) + dont_care_bits = max_enc_bits - (flat_enc_mask[1] - flat_enc_mask[0]) + if dont_care_bits < 0: + raise IsaAdditionError( + 'encoding identifier mask exceeds the primary decode width' + ) + return flat_enc_mask, op_mask, dont_care_bits + + +def _required_text(parent: elem_tree.Element, tag: str, context: str) -> str: + node = parent.find(tag) + if node is None or node.text is None or not node.text.strip(): + raise IsaAdditionError(f'{context}: missing non-empty <{tag}>') + return node.text.strip() + + +def _required_decimal(text: str, context: str) -> int: + try: + value = int(text) + except ValueError as error: + raise IsaAdditionError(f'{context}: invalid decimal value {text!r}') from error + return value + + +def _binary_text(node: elem_tree.Element, bit_width: int, context: str) -> str: + unknown_attributes = set(node.attrib) - {xs.ENC_IDENTIFER_ATTR_RADIX} + if unknown_attributes: + names = ', '.join(sorted(unknown_attributes)) + raise IsaAdditionError(f'{context}: unknown attributes: {names}') + radix_text = node.attrib.get(xs.ENC_IDENTIFER_ATTR_RADIX) + if radix_text is None: + raise IsaAdditionError(f'{context}: missing Radix attribute') + radix = _required_decimal(radix_text, f'{context} Radix') + if radix != 2: + raise IsaAdditionError( + f'{context}: radix {radix} does not match binary MR ISA layout' + ) + if node.text is None or not node.text.strip(): + raise IsaAdditionError(f'{context}: missing identifier value') + value = node.text.strip() + if not re.fullmatch(r'[01]+', value): + raise IsaAdditionError(f'{context}: malformed radix-2 identifier {value!r}') + if len(value) != bit_width: + raise IsaAdditionError( + f'{context}: identifier width {len(value)} does not match encoding ' + f'width {bit_width}' + ) + return value + + +def _field_widths( + encoding: elem_tree.Element, profile: IsaProfile, context: str +) -> tuple[int, int, int]: + enc_name = _required_text(encoding, xs.ENCODING_NAME, context) + renames = profile.field_renames(enc_name.upper()) + enc_field_bits: int | None = None + op_field_bits = 0 + opm_field_bits = 0 + for field in encoding.findall(f'./{xs.UCODE_FMT}/{xs.BITMAP}/{xs.FIELD}'): + field_name = _required_text(field, xs.FIELD_NAME, context).lower() + field_name = renames.get(field_name, field_name) + ranges = sorted( + field.findall(f'{xs.BIT_LAYOUT}/{xs.RANGE}'), + key=lambda node: int(node.attrib.get('Order', 0)), + ) + for range_index, range_node in enumerate(ranges): + width = _required_decimal( + _required_text(range_node, xs.BIT_CNT, context), context + ) + range_name = field_name + if range_index > 0: + range_name = 'opm' if field_name == 'op' and range_index == 1 else '' + if range_name == 'encoding': + enc_field_bits = width + elif range_name == 'op': + op_field_bits = width + elif range_name == 'opm': + opm_field_bits = width + if enc_field_bits is None: + raise IsaAdditionError(f'{context}: microcode format has no encoding field') + return enc_field_bits, op_field_bits, opm_field_bits + + +def _layout_signature(text: str, opcode_slice: tuple[int, int]) -> str: + start, end = opcode_slice + return f'{text[:start]}{"0" * (end - start)}{text[end:]}' + + +def _decode_identifier( + text: str, + encoding_slice: tuple[int, int], + opcode_slice: tuple[int, int], + dont_care_bits: int, +) -> tuple[int, int]: + encoding_value = int(text[encoding_slice[0] : encoding_slice[1]], 2) + encoding_value <<= dont_care_bits + opcode_text = text[opcode_slice[0] : opcode_slice[1]] + opcode = int(opcode_text, 2) if opcode_text else 0 + return encoding_value, opcode + + +def _base_metadata( + root: elem_tree.Element, +) -> tuple[str, str, elem_tree.Element, elem_tree.Element]: + try: + isa_node = xs.get_node(root, xs.ISA) + arch_node = xs.get_node(isa_node, xs.ARCH) + arch_name = xs.get_node_text(xs.get_node(arch_node, xs.ARCH_NAME)).strip() + document = xs.get_node(root, xs.DOCUMENT) + schema_version = xs.get_node_text( + xs.get_node(document, xs.SCHEMA_VERSION) + ).strip() + instructions = xs.get_node(isa_node, xs.INSTS) + encodings = xs.get_node(isa_node, xs.ENCODINGS) + except (xs.SchemaValueError, AttributeError) as error: + raise IsaAdditionError( + f'base MR ISA XML is missing required metadata: {error}' + ) from error + return arch_name, schema_version, encodings, instructions + + +def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ + dict[str, _EncodingDefinition], + set[str], + dict[tuple[str, int], str], + set[str], + set[str], +]: + isa_node = xs.get_node(root, xs.ISA) + encodings_node = xs.get_node(isa_node, xs.ENCODINGS) + instructions_node = xs.get_node(isa_node, xs.INSTS) + operand_types_node = xs.get_node(isa_node, xs.OPERAND_TYPES) + + encoding_names = { + _required_text(encoding, xs.ENCODING_NAME, 'base encoding') + for encoding in encodings_node + } + encoding_definitions: dict[str, _EncodingDefinition] = {} + for encoding in encodings_node: + context = 'base encoding' + name = _required_text(encoding, xs.ENCODING_NAME, context) + if name in profile.skip_encodings: + continue + if name in encoding_definitions: + raise IsaAdditionError(f'base encoding {name!r} is duplicated') + bit_width = _required_decimal( + _required_text(encoding, xs.BIT_CNT, f'base encoding {name!r}'), + f'base encoding {name!r} bit width', + ) + mask_node = encoding.find(xs.ENCODING_IDENTIFIER_MASK) + if mask_node is None: + raise IsaAdditionError( + f'base encoding {name!r}: missing <{xs.ENCODING_IDENTIFIER_MASK}>' + ) + mask = _binary_text(mask_node, bit_width, f'base encoding {name!r} mask') + identifiers_node = encoding.find(xs.ENCODING_IDENTIFERS) + if identifiers_node is None: + raise IsaAdditionError( + f'base encoding {name!r}: missing <{xs.ENCODING_IDENTIFERS}>' + ) + enc_bits, op_bits, opm_bits = _field_widths( + encoding, profile, f'base encoding {name!r}' + ) + try: + encoding_slice, opcode_slice, dont_care_bits = ( + parse_encoding_identifier_mask( + mask, profile.max_enc_bits, enc_bits, op_bits + ) + ) + except IsaAdditionError as error: + raise IsaAdditionError(f'base encoding {name!r}: {error}') from error + if encoding_slice[1] > bit_width or opcode_slice[1] > bit_width: + raise IsaAdditionError( + f'base encoding {name!r}: identifier mask slices exceed bit width ' + f'{bit_width}' + ) + + texts: set[str] = set() + layout_signatures: set[str] = set() + encoding_values: set[int] = set() + decoded_opcodes: set[int] = set() + for identifier in identifiers_node: + text = _binary_text( + identifier, bit_width, f'base encoding {name!r} identifier' + ) + encoding_value, opcode = _decode_identifier( + text, encoding_slice, opcode_slice, dont_care_bits + ) + texts.add(text) + layout_signatures.add(_layout_signature(text, opcode_slice)) + encoding_values.add(encoding_value) + decoded_opcodes.add(opcode) + + parent_name: str | None = None + is_implied_literal = False + if profile.is_alt_encoding(name): + parent_name = profile.derive_parent_enc_name(name) + parent = encoding_definitions.get(parent_name) + if parent is None: + raise IsaAdditionError( + f'base encoding {name!r}: parent {parent_name!r} must appear first' + ) + condition_names = [ + (_required_text(condition, xs.COND_NAME, f'base encoding {name!r}'), '') + for condition in encoding.findall( + f'./{xs.ENCODING_CONDS}/{xs.ENCODING_COND}' + ) + ] + is_implied_literal = profile.is_implied_literal_encoding( + name, condition_names, bit_width, parent.bit_width + ) + + encoding_definitions[name] = _EncodingDefinition( + name=name, + identifiers_node=identifiers_node, + bit_width=bit_width, + radix=2, + flat_encoding_slice=encoding_slice, + opcode_slice=opcode_slice, + dont_care_bits=dont_care_bits, + opcode_bit_count=op_bits, + opcode_modifier_bit_count=opm_bits, + base_layout_signatures=frozenset(layout_signatures), + base_encoding_values=frozenset(encoding_values), + identifier_texts=frozenset(texts), + decoded_opcodes=frozenset(decoded_opcodes), + parent_name=parent_name, + is_implied_literal=is_implied_literal, + ) + operand_types = { + _required_text(node, xs.OPERAND_TYPE_NAME, 'base operand type') + for node in operand_types_node + } + instruction_names: set[str] = set() + opcode_owners: dict[tuple[str, int], str] = {} + for instruction in instructions_node: + name = _required_text(instruction, xs.INST_NAME, 'base instruction') + instruction_names.add(name) + encodings = instruction.find(xs.INST_ENCODINGS) + if encodings is None: + continue + for encoding in encodings: + enc_name = _required_text( + encoding, xs.ENCODING_NAME, f'base instruction {name}' + ) + opcode_text = _required_text( + encoding, xs.OPCODE, f'base instruction {name}/{enc_name}' + ) + try: + opcode = int(opcode_text) + except ValueError as error: + raise IsaAdditionError( + f'base instruction {name}/{enc_name}: invalid decimal opcode ' + f'{opcode_text!r}' + ) from error + opcode_owners.setdefault((enc_name, opcode), name) + return ( + encoding_definitions, + encoding_names, + opcode_owners, + operand_types, + instruction_names, + ) + + +def _parse_additions(path: str) -> elem_tree.Element: + try: + return elem_tree.parse(path).getroot() + except (OSError, elem_tree.ParseError) as error: + raise IsaAdditionError( + f'{path}: cannot parse ISA additions document: {error}' + ) from error + + +def _validate_additions_root( + root: elem_tree.Element, path: str, base_arch: str, base_schema: str +) -> tuple[str, elem_tree.Element | None, elem_tree.Element]: + if root.tag != ADDITIONS_ROOT: + raise IsaAdditionError( + f'{path}: expected <{ADDITIONS_ROOT}> root, found <{root.tag}>' + ) + unknown_attributes = set(root.attrib) - _ADDITIONS_ATTRIBUTES + missing_attributes = _ADDITIONS_ATTRIBUTES - set(root.attrib) + if unknown_attributes: + names = ', '.join(sorted(unknown_attributes)) + raise IsaAdditionError(f'{path}: unknown additions root attributes: {names}') + if missing_attributes: + names = ', '.join(sorted(missing_attributes)) + raise IsaAdditionError(f'{path}: missing additions root attributes: {names}') + + identifier = root.attrib[ADDITIONS_ID_ATTR].strip() + if not _ID_PATTERN.fullmatch(identifier): + raise IsaAdditionError( + f'{path}: invalid additions document Id {identifier!r}; use letters, ' + 'digits, dot, underscore, or hyphen' + ) + + declared_arch = root.attrib[ADDITIONS_BASE_ARCH_ATTR].strip() + if declared_arch != base_arch: + raise IsaAdditionError( + f'{path}: additions base architecture {declared_arch!r} does not match ' + f'{base_arch!r}' + ) + declared_schema = root.attrib[ADDITIONS_BASE_SCHEMA_ATTR].strip() + if declared_schema != base_schema: + raise IsaAdditionError( + f'{path}: additions base schema {declared_schema!r} does not match ' + f'{base_schema!r}' + ) + + children = list(root) + allowed_children = {IDENTIFIER_ADDITIONS, INSTRUCTION_ADDITIONS} + unknown_children = [ + child.tag for child in children if child.tag not in allowed_children + ] + if unknown_children: + names = ', '.join(f'<{name}>' for name in unknown_children) + raise IsaAdditionError(f'{path}: unknown additions root elements: {names}') + instructions = [child for child in children if child.tag == INSTRUCTION_ADDITIONS] + identifier_additions = [ + child for child in children if child.tag == IDENTIFIER_ADDITIONS + ] + if len(instructions) != 1: + raise IsaAdditionError( + f'{path}: additions document must contain exactly one ' + f'<{INSTRUCTION_ADDITIONS}> element' + ) + if len(identifier_additions) > 1: + raise IsaAdditionError( + f'{path}: additions document may contain at most one ' + f'<{IDENTIFIER_ADDITIONS}> element' + ) + instructions_node = instructions[0] + if instructions_node.attrib: + names = ', '.join(sorted(instructions_node.attrib)) + raise IsaAdditionError( + f'{path}: <{INSTRUCTION_ADDITIONS}> must not have attributes: {names}' + ) + identifier_group = identifier_additions[0] if identifier_additions else None + if identifier_group is not None and identifier_group.attrib: + names = ', '.join(sorted(identifier_group.attrib)) + raise IsaAdditionError( + f'{path}: <{IDENTIFIER_ADDITIONS}> must not have attributes: {names}' + ) + if identifier_group is not None and not list(identifier_group): + raise IsaAdditionError(f'{path}: <{IDENTIFIER_ADDITIONS}> must not be empty') + return identifier, identifier_group, instructions_node + + +def _validate_instruction_addition( + instruction: elem_tree.Element, + *, + path: str, + addition_id: str, + encoding_names: set[str], + opcode_owners: dict[tuple[str, int], str], + opcode_limits: Mapping[str, int], + operand_types: set[str], + instruction_names: set[str], +) -> _InstructionAddition: + if instruction.tag != xs.INST: + raise IsaAdditionError( + f'{path}: <{INSTRUCTION_ADDITIONS}> may contain only <{xs.INST}> ' + f'elements, found <{instruction.tag}>' + ) + + context = f'{path}: additions document {addition_id!r}' + name = _required_text(instruction, xs.INST_NAME, context) + if name in instruction_names: + raise IsaAdditionError(f'{context}: instruction {name!r} already exists') + + instruction_encodings = instruction.find(xs.INST_ENCODINGS) + if instruction_encodings is None or not list(instruction_encodings): + raise IsaAdditionError( + f'{context}: instruction {name!r} has no <{xs.INST_ENCODING}> entries' + ) + + forms_in_order: list[tuple[str, int, str]] = [] + forms: set[tuple[str, int, str]] = set() + for encoding in instruction_encodings: + if encoding.tag != xs.INST_ENCODING: + raise IsaAdditionError( + f'{context}: <{xs.INST_ENCODINGS}> may contain only ' + f'<{xs.INST_ENCODING}> elements, found <{encoding.tag}>' + ) + enc_name = _required_text( + encoding, xs.ENCODING_NAME, f'{context}: instruction {name!r}' + ) + if enc_name not in encoding_names: + raise IsaAdditionError( + f'{context}: instruction {name!r} references unknown encoding ' + f'{enc_name!r}' + ) + opcode_text = _required_text( + encoding, xs.OPCODE, f'{context}: instruction {name!r}/{enc_name}' + ) + try: + opcode = int(opcode_text) + except ValueError as error: + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} has invalid decimal ' + f'opcode {opcode_text!r}' + ) from error + if opcode < 0: + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} has negative opcode ' + f'{opcode}' + ) + opcode_limit = opcode_limits.get(enc_name) + if opcode_limit is not None and opcode >= opcode_limit: + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} opcode {opcode} is ' + f'outside the encoding range [0, {opcode_limit})' + ) + + slot = (enc_name, opcode) + owner = opcode_owners.get(slot) + if owner is not None: + raise IsaAdditionError( + f'{context}: opcode {opcode} in encoding {enc_name!r} is already ' + f'owned by instruction {owner!r}' + ) + condition = _required_text( + encoding, + xs.ENCODING_COND, + f'{context}: instruction {name!r}/{enc_name}', + ) + form = (enc_name, opcode, condition) + if form in forms: + raise IsaAdditionError( + f'{context}: instruction {name!r} repeats opcode {opcode} in ' + f'encoding {enc_name!r} with condition {condition!r}' + ) + forms.add(form) + forms_in_order.append(form) + + operands = encoding.find(xs.OPERANDS) + if operands is None: + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} is missing ' + f'<{xs.OPERANDS}>' + ) + for operand in operands: + op_type = _required_text( + operand, + xs.OPERAND_TYPE, + f'{context}: instruction {name!r}/{enc_name} operand', + ) + if op_type not in operand_types: + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} references ' + f'unknown operand type {op_type!r}' + ) + + addition = deepcopy(instruction) + addition.attrib[ADDITION_SOURCE_ATTR] = addition_id + return _InstructionAddition( + node=addition, + name=name, + path=path, + addition_id=addition_id, + forms=tuple(forms_in_order), + ) + + +def _single_child( + parent: elem_tree.Element, tag: str, context: str +) -> elem_tree.Element: + nodes = [child for child in parent if child.tag == tag] + if len(nodes) != 1: + raise IsaAdditionError(f'{context}: expected exactly one <{tag}> element') + return nodes[0] + + +def _validate_identifier_addition( + addition: elem_tree.Element, + *, + path: str, + addition_id: str, + encodings: Mapping[str, _EncodingDefinition], + all_encoding_names: set[str], + occupied_texts: dict[str, set[str]], + occupied_opcodes: dict[str, set[int]], +) -> _IdentifierAddition: + context = f'{path}: additions document {addition_id!r} identifier addition' + if addition.tag != IDENTIFIER_ADDITION: + raise IsaAdditionError( + f'{path}: <{IDENTIFIER_ADDITIONS}> may contain only ' + f'<{IDENTIFIER_ADDITION}> elements, found <{addition.tag}>' + ) + if addition.attrib: + names = ', '.join(sorted(addition.attrib)) + raise IsaAdditionError(f'{context}: unknown attributes: {names}') + allowed_children = {xs.ENCODING_NAME, xs.OPCODE, xs.ENCODING_IDENTIFER_ALT} + unknown_children = [ + child.tag for child in addition if child.tag not in allowed_children + ] + if unknown_children: + names = ', '.join(f'<{name}>' for name in unknown_children) + raise IsaAdditionError(f'{context}: unknown elements: {names}') + + encoding_name_node = _single_child(addition, xs.ENCODING_NAME, context) + opcode_node = _single_child(addition, xs.OPCODE, context) + identifier_node = _single_child(addition, xs.ENCODING_IDENTIFER_ALT, context) + if encoding_name_node.attrib or opcode_node.attrib: + raise IsaAdditionError( + f'{context}: <{xs.ENCODING_NAME}> and <{xs.OPCODE}> must not ' + 'have attributes' + ) + + encoding_name = _required_text(addition, xs.ENCODING_NAME, context) + if encoding_name not in all_encoding_names: + raise IsaAdditionError(f'{context}: unknown encoding {encoding_name!r}') + encoding = encodings.get(encoding_name) + if encoding is None: + raise IsaAdditionError( + f'{context}: encoding {encoding_name!r} is not active in this ISA profile' + ) + opcode_text = _required_text(addition, xs.OPCODE, context) + opcode = _required_decimal(opcode_text, f'{context} opcode') + if opcode < 0 or opcode >= encoding.opcode_limit: + raise IsaAdditionError( + f'{context}: opcode {opcode} is outside encoding {encoding_name!r} ' + f'range [0, {encoding.opcode_limit})' + ) + + radix_text = identifier_node.attrib.get(xs.ENC_IDENTIFER_ATTR_RADIX) + if radix_text is None: + raise IsaAdditionError(f'{context}: is missing Radix') + radix = _required_decimal(radix_text, f'{context} identifier Radix') + if radix != encoding.radix: + raise IsaAdditionError( + f'{context}: identifier radix {radix} does not match encoding ' + f'radix {encoding.radix}' + ) + text = _binary_text(identifier_node, encoding.bit_width, context) + encoding_value, decoded_opcode = _decode_identifier( + text, + encoding.flat_encoding_slice, + encoding.opcode_slice, + encoding.dont_care_bits, + ) + if decoded_opcode != opcode: + raise IsaAdditionError( + f'{context}: identifier decodes opcode {decoded_opcode}, not declared ' + f'opcode {opcode} for encoding {encoding_name!r}' + ) + if ( + encoding_value not in encoding.base_encoding_values + or _layout_signature(text, encoding.opcode_slice) + not in encoding.base_layout_signatures + ): + raise IsaAdditionError( + f'{context}: identifier for encoding {encoding_name!r} is ' + 'incompatible with the base identifier mask/layout' + ) + if text in occupied_texts[encoding_name]: + raise IsaAdditionError( + f'{context}: duplicate identifier for encoding {encoding_name!r}' + ) + if opcode in occupied_opcodes[encoding_name]: + raise IsaAdditionError( + f'{context}: identifier collides with an existing decode slot for ' + f'encoding {encoding_name!r} opcode {opcode}' + ) + + copied_identifier = deepcopy(identifier_node) + copied_identifier.attrib[ADDITION_SOURCE_ATTR] = addition_id + occupied_texts[encoding_name].add(text) + occupied_opcodes[encoding_name].add(opcode) + return _IdentifierAddition( + node=copied_identifier, + encoding=encoding, + opcode=opcode, + ) + + +def _reachable_opcodes( + encodings: Mapping[str, _EncodingDefinition], + identifiers: Sequence[_IdentifierAddition], +) -> dict[str, set[int]]: + reachable = { + name: set(encoding.decoded_opcodes) for name, encoding in encodings.items() + } + for addition in identifiers: + reachable[addition.encoding.name].add(addition.opcode) + for name, encoding in encodings.items(): + if encoding.opcode_modifier_bit_count == 0: + continue + base_count = 1 << encoding.opcode_bit_count + base_opcodes = tuple(reachable[name]) + for modifier in range(1, 1 << encoding.opcode_modifier_bit_count): + reachable[name].update( + opcode + modifier * base_count for opcode in base_opcodes + ) + return reachable + + +def apply_isa_additions( + base_root: elem_tree.Element, + addition_paths: Sequence[str], + profile: IsaProfile, +) -> tuple[IsaAdditionProvenance, ...]: + """Validate and atomically merge ISA additions into a base MR ISA tree. + + Validation of every input completes before the base tree is modified. The + returned provenance and merge order follow ``addition_paths`` and document + order. Identifier additions are merged before the caller constructs normal + decode tables. + """ + if not addition_paths: + return () + + base_arch, base_schema, _, base_instructions = _base_metadata(base_root) + ( + encodings, + encoding_names, + opcode_owners, + operand_types, + instruction_names, + ) = _base_definitions(base_root, profile) + opcode_limits = { + name: encoding.opcode_limit for name, encoding in encodings.items() + } + pending_instructions: list[_InstructionAddition] = [] + pending_identifier_groups: list[tuple[str, str, elem_tree.Element]] = [] + pending_identifiers: list[_IdentifierAddition] = [] + addition_opcode_owners: dict[tuple[str, int], str] = {} + provenance: list[IsaAdditionProvenance] = [] + addition_ids: set[str] = set() + + for raw_path in addition_paths: + path = str(raw_path) + addition_root = _parse_additions(path) + addition_id, identifier_additions, instruction_additions = ( + _validate_additions_root(addition_root, path, base_arch, base_schema) + ) + if addition_id in addition_ids: + raise IsaAdditionError( + f'{path}: duplicate additions document Id {addition_id!r}' + ) + + addition_ids.add(addition_id) + provenance.append(IsaAdditionProvenance(addition_id, path)) + if identifier_additions is not None: + pending_identifier_groups.append((path, addition_id, identifier_additions)) + for instruction in instruction_additions: + addition = _validate_instruction_addition( + instruction, + path=path, + addition_id=addition_id, + encoding_names=encoding_names, + opcode_owners=opcode_owners, + opcode_limits=opcode_limits, + operand_types=operand_types, + instruction_names=instruction_names, + ) + instruction_names.add(addition.name) + for enc_name, opcode, _ in addition.forms: + slot = (enc_name, opcode) + opcode_owners[slot] = addition.name + addition_opcode_owners[slot] = addition.name + pending_instructions.append(addition) + + occupied_texts = { + name: set(encoding.identifier_texts) for name, encoding in encodings.items() + } + occupied_opcodes = { + name: set(encoding.decoded_opcodes) for name, encoding in encodings.items() + } + for path, addition_id, identifier_group in pending_identifier_groups: + for identifier in identifier_group: + addition = _validate_identifier_addition( + identifier, + path=path, + addition_id=addition_id, + encodings=encodings, + all_encoding_names=encoding_names, + occupied_texts=occupied_texts, + occupied_opcodes=occupied_opcodes, + ) + owner = addition_opcode_owners.get( + (addition.encoding.name, addition.opcode) + ) + if owner is None: + raise IsaAdditionError( + f'{path}: additions document {addition_id!r} identifier for encoding ' + f'{addition.encoding.name!r} opcode {addition.opcode} is ' + 'unowned; it must correspond to an added instruction' + ) + if addition.encoding.is_implied_literal: + parent_name = addition.encoding.parent_name + parent_owner = addition_opcode_owners.get( + (parent_name, addition.opcode) + ) + if parent_owner != owner: + parent_owner_text = ( + repr(parent_owner) if parent_owner is not None else 'no owner' + ) + raise IsaAdditionError( + f'{path}: additions document {addition_id!r} ' + 'implied-literal identifier ' + f'for {addition.encoding.name!r} opcode {addition.opcode} ' + f'is owned by {owner!r}, but parent encoding ' + f'{parent_name!r} has {parent_owner_text}' + ) + pending_identifiers.append(addition) + + reachable = _reachable_opcodes(encodings, pending_identifiers) + for addition in pending_instructions: + for enc_name, opcode, condition in addition.forms: + if enc_name in profile.skip_encodings or profile.skip_inst_encoding( + enc_name, condition + ): + continue + if opcode not in reachable[enc_name]: + raise IsaAdditionError( + f'{addition.path}: additions document ' + f'{addition.addition_id!r} instruction ' + f'{addition.name!r} encoding {enc_name!r} opcode {opcode} is ' + 'unreachable; add a validated encoding identifier' + ) + + for addition in pending_identifiers: + addition.encoding.identifiers_node.append(addition.node) + base_instructions.extend(addition.node for addition in pending_instructions) + return tuple(provenance) diff --git a/emulation/rocjitsu/lib/python/amdisa/parser.py b/emulation/rocjitsu/lib/python/amdisa/parser.py index 13fd78fe35e5..f0324bca6d41 100644 --- a/emulation/rocjitsu/lib/python/amdisa/parser.py +++ b/emulation/rocjitsu/lib/python/amdisa/parser.py @@ -33,10 +33,10 @@ by checking instruction semantics that require the old value. """ +from collections.abc import Sequence import re import xml.etree.ElementTree as elem_tree - from amdisa import xml_schema as xs from amdisa.gpuisa import ( DecodeTableEntry, @@ -49,6 +49,12 @@ OperandSelector, synthesize_fieldless_name, ) +from amdisa.isa_additions import ( + ADDITION_SOURCE_ATTR, + IsaAdditionError, + apply_isa_additions, + parse_encoding_identifier_mask as _parse_enc_id_masks, +) from amdisa.fieldless_policy import validate_fieldless_taxonomy from amdisa.isa_profile import IsaProfile @@ -93,50 +99,6 @@ def _fill_padding_gaps( return pads -def _parse_enc_id_masks( - enc_id_mask: str, - max_enc_bits: int, - enc_field_bit_cnt: int, - op_field_bit_cnt: int, -) -> tuple[tuple[int, int], tuple[int, int], int]: - """Parse an encoding identifier mask into (flat_enc_mask, op_mask, dont_care_bits). - - The encoding identifier mask is a binary string where '1' bits mark the - encoding field and '0' bits separate encoding from opcode. This function - derives the slice positions for the encoding field and opcode field, plus - the number of don't-care bits available in the primary decode table index. - - Args: - enc_id_mask: Binary string identifier mask (e.g. '111111111000000000000000'). - max_enc_bits: Maximum bits for the primary decode table index. - enc_field_bit_cnt: Bit width of the encoding identifier field. - op_field_bit_cnt: Bit width of the opcode field. - - Returns: - Tuple of (flat_enc_mask, op_mask, dont_care_bits) where each mask is - a (start, end) slice pair into the identifier string. - """ - bit_masks = [(x.start(), x.end()) for x in re.finditer(r'1+', enc_id_mask)] - flat_enc_mask = bit_masks[0] - if len(bit_masks) == 1: - op_mask = ( - bit_masks[0][0] + enc_field_bit_cnt, - bit_masks[0][0] + enc_field_bit_cnt + op_field_bit_cnt, - ) - if (flat_enc_mask[1] - flat_enc_mask[0]) > max_enc_bits: - flat_enc_mask = ( - flat_enc_mask[0], - flat_enc_mask[0] + max_enc_bits, - ) - else: - op_mask = ( - bit_masks[1][0], - bit_masks[1][0] + op_field_bit_cnt, - ) - dont_care_bits = max_enc_bits - (flat_enc_mask[1] - flat_enc_mask[0]) - return flat_enc_mask, op_mask, dont_care_bits - - def _uniquify_fieldless_names(opnds: list[Operand]) -> None: """Make fieldless operand names unique within one instruction, in place. @@ -331,6 +293,8 @@ class Parser: Attributes: isa_xml: Path to XML file for the ISA specification. profile: ISA-specific encoding rules used during parsing. + addition_xmls: Ordered ISA additions XML paths to validate and merge + before decode-table and instruction parsing. tree: XML element tree obtained by parsing the XML file. root: Root of the XML element tree. isa_spec: ISA specification object. @@ -339,7 +303,12 @@ class Parser: operand_types_node: Element tree node pointing to the operand types. """ - def __init__(self, isa_xml: str, profile: IsaProfile) -> None: + def __init__( + self, + isa_xml: str, + profile: IsaProfile, + addition_xmls: Sequence[str] = (), + ) -> None: self.isa_xml = isa_xml self.profile = profile self.tree = elem_tree.parse(self.isa_xml) @@ -366,6 +335,8 @@ def __init__(self, isa_xml: str, profile: IsaProfile) -> None: generated_dir_name, cpp_namespace, ) + self.addition_xmls = tuple(addition_xmls) + self._addition_by_id = {} self.encodings_node = xs.get_node(isa_node, xs.ENCODINGS) self.insts_node = xs.get_node(isa_node, xs.INSTS) @@ -377,14 +348,55 @@ def parse(self) -> IsaSpec: Returns: Populated IsaSpec object. """ + self.isa_spec.applied_additions = apply_isa_additions( + self.root, self.addition_xmls, self.profile + ) + self._addition_by_id = { + addition.identifier: addition + for addition in self.isa_spec.applied_additions + } self.parse_encodings() self.parse_insts() + self._validate_addition_decode_reachability() self.parse_operand_types() self._inject_compat_insts() self._collect_fieldless_operand_types() validate_fieldless_taxonomy(self.isa_spec) return self.isa_spec + def _validate_addition_decode_reachability(self) -> None: + """Check the final decode pointers for every active added instruction form.""" + for inst_node in self.insts_node: + addition_id = inst_node.attrib.get(ADDITION_SOURCE_ATTR) + if addition_id is None: + continue + provenance = self._addition_by_id[addition_id] + inst_name = xs.get_node_text(xs.get_node(inst_node, xs.INST_NAME)) + for inst_enc_node in xs.get_node(inst_node, xs.INST_ENCODINGS): + enc_name = xs.get_node_text( + xs.get_node(inst_enc_node, xs.ENCODING_NAME) + ) + if enc_name in self.profile.skip_encodings: + continue + condition = xs.get_node_text( + xs.get_node(inst_enc_node, xs.ENCODING_COND) + ) + if self.profile.skip_inst_encoding(enc_name, condition): + continue + opcode = int(xs.get_node_text(xs.get_node(inst_enc_node, xs.OPCODE))) + pointers = self.isa_spec.encoding_map[enc_name].primary_dt_ptrs + if ( + pointers is None + or opcode >= len(pointers) + or pointers[opcode] == -1 + ): + raise IsaAdditionError( + f'{provenance.path}: additions document {addition_id!r} ' + f'instruction {inst_name!r} encoding {enc_name!r} ' + f'opcode {opcode} is ' + 'unreachable in the final primary decode table' + ) + def implicit_operand_accesses( self, operand_type: str ) -> dict[tuple[str, str], tuple[bool, bool]]: @@ -1176,6 +1188,10 @@ class with a literal constant). inst_name_node = xs.get_node(inst_node, xs.INST_NAME) inst_encs_node = xs.get_node(inst_node, xs.INST_ENCODINGS) inst_name = inst_name_node.text + addition_id = inst_node.attrib.get(ADDITION_SOURCE_ATTR) + source_addition = ( + self._addition_by_id.get(addition_id) if addition_id else None + ) available_encodings = frozenset( xs.get_node_text(xs.get_node(inst_enc_node, xs.ENCODING_NAME)) for inst_enc_node in inst_encs_node @@ -1261,6 +1277,7 @@ class with a literal constant). opnds, is_implied_literal, available_encodings, + source_addition, ) # Implied-literal instructions go to the parent encoding's diff --git a/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py new file mode 100644 index 000000000000..b0936fa5ae46 --- /dev/null +++ b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py @@ -0,0 +1,983 @@ +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# SPDX-License-Identifier: MIT + +from copy import deepcopy +import re +import subprocess +import sys +import xml.etree.ElementTree as elem_tree +from pathlib import Path + +import pytest + +from amdisa.__main__ import _group_isa_additions_args +from amdisa.isa_additions import ( + ADDITION_SOURCE_ATTR, + IsaAdditionError, + apply_isa_additions, +) +from amdisa.isa_profile import Cdna5Profile +from amdisa.parser import Parser + +_BASE_XML = '''\ + + 1.2.0 + + + AMD TEST 1 + 1 + + + + ENC_TEST + 32 + 11110000111000000000000000000000 + + 10110000000000000000000000000000 + 10100000001000000000000000000000 + 10100000010000000000000000000000 + 10100000010000000000000000000001 + + + + + ENCODING + 428 + + + OP + 321 + + + + + + + + BASE_INST + + + ENC_TEST + default + 1 + + OPR_TEST + + + + + + + OPR_TEST + + + +''' + +_PROFILE = Cdna5Profile() + + +def _identifier_text( + opcode: int, + *, + alternate_layout: bool = False, + alternate_encoding_value: bool = False, +) -> str: + encoding = '1011' if alternate_encoding_value else '1010' + return f'{encoding}0000{opcode:03b}{"0" * 20}{int(alternate_layout)}' + + +def _identifier_addition( + opcode: int, + *, + encoding: str = 'ENC_TEST', + declared_opcode: int | str | None = None, + radix: int | str = 2, + text: str | None = None, +) -> str: + if declared_opcode is None: + declared_opcode = opcode + if text is None: + text = _identifier_text(opcode) + return f'''\ + + {encoding} + {declared_opcode} + {text} + +''' + + +def _instruction( + name: str = 'ADDITION_INST', + encoding: str = 'ENC_TEST', + opcode: int | str = 2, + operand_type: str = 'OPR_TEST', +) -> str: + return f'''\ + + {name} + + + {encoding} + default + {opcode} + + {operand_type} + + + + +''' + + +def _write_additions( + tmp_path: Path, + instructions: str = '', + *, + identifier: str = 'test-additions', + architecture: str = 'AMD TEST 1', + schema: str = '1.2.0', + filename: str = 'additions.xml', + extra_root: str = '', + extra_attributes: str = '', + identifier_additions: str | None = None, + instructions_attributes: str = '', + identifier_container_attributes: str = '', +) -> Path: + identifiers = '' + if identifier_additions is not None: + identifiers = f'''\ + + {identifier_additions} + ''' + path = tmp_path / filename + path.write_text(f'''\ + + {identifiers} + + {instructions} + + {extra_root} + +''') + return path + + +def _base_root() -> elem_tree.Element: + return elem_tree.fromstring(_BASE_XML) + + +def test_empty_additions_document_does_not_mutate_base_xml(tmp_path): + root = _base_root() + before = elem_tree.tostring(root) + addition = _write_additions(tmp_path) + + provenance = apply_isa_additions(root, [str(addition)], _PROFILE) + + assert elem_tree.tostring(root) == before + assert [(item.identifier, item.path) for item in provenance] == [ + ('test-additions', str(addition)) + ] + + +def test_addition_is_appended_with_provenance(tmp_path): + root = _base_root() + addition = _write_additions(tmp_path, _instruction()) + + apply_isa_additions(root, [str(addition)], _PROFILE) + + instructions = root.findall('./ISA/Instructions/Instruction') + assert [node.findtext('InstructionName') for node in instructions] == [ + 'BASE_INST', + 'ADDITION_INST', + ] + assert instructions[0].get(ADDITION_SOURCE_ATTR) is None + assert instructions[1].get(ADDITION_SOURCE_ATTR) == 'test-additions' + + +def test_instruction_using_existing_populated_identifier_succeeds(tmp_path): + root = _base_root() + addition = _write_additions(tmp_path, _instruction(opcode=2)) + + apply_isa_additions(root, [str(addition)], _PROFILE) + + assert ( + root.findall('./ISA/Instructions/Instruction')[-1].findtext('InstructionName') + == 'ADDITION_INST' + ) + + +def test_instruction_and_primary_identifier_are_merged_atomically(tmp_path): + root = _base_root() + addition = _write_additions( + tmp_path, + _instruction(opcode=3), + identifier_additions=_identifier_addition(3), + ) + + apply_isa_additions(root, [str(addition)], _PROFILE) + + identifiers = root.findall( + './ISA/Encodings/Encoding/EncodingIdentifiers/EncodingIdentifier' + ) + assert identifiers[-1].text == _identifier_text(3) + assert identifiers[-1].get(ADDITION_SOURCE_ATTR) == 'test-additions' + assert ( + root.findall('./ISA/Instructions/Instruction')[-1].get(ADDITION_SOURCE_ATTR) + == 'test-additions' + ) + + +def test_multiple_additions_preserve_command_line_and_document_order(tmp_path): + first = _write_additions( + tmp_path, + _instruction('ADDITION_A', opcode=2) + _instruction('ADDITION_B', opcode=3), + identifier='first', + filename='first.xml', + identifier_additions=_identifier_addition(3), + ) + second = _write_additions( + tmp_path, + _instruction('ADDITION_C', opcode=4), + identifier='second', + filename='second.xml', + identifier_additions=_identifier_addition(4), + ) + left = _base_root() + right = _base_root() + + left_provenance = apply_isa_additions(left, [str(first), str(second)], _PROFILE) + right_provenance = apply_isa_additions(right, [str(first), str(second)], _PROFILE) + + assert elem_tree.tostring(left) == elem_tree.tostring(right) + assert [item.identifier for item in left_provenance] == ['first', 'second'] + assert left_provenance == right_provenance + assert [ + node.findtext('InstructionName') + for node in left.findall('./ISA/Instructions/Instruction') + ] == ['BASE_INST', 'ADDITION_A', 'ADDITION_B', 'ADDITION_C'] + assert [ + node.text + for node in left.findall( + './ISA/Encodings/Encoding/EncodingIdentifiers/EncodingIdentifier' + )[-2:] + ] == [_identifier_text(3), _identifier_text(4)] + + +def test_all_additions_validate_before_base_is_modified(tmp_path): + valid = _write_additions( + tmp_path, + _instruction('VALID', opcode=2), + identifier='valid', + filename='valid.xml', + ) + invalid = _write_additions( + tmp_path, + _instruction('INVALID', encoding='ENC_MISSING', opcode=3), + identifier='invalid', + filename='invalid.xml', + ) + root = _base_root() + before = elem_tree.tostring(root) + + with pytest.raises(IsaAdditionError, match='unknown encoding'): + apply_isa_additions(root, [str(valid), str(invalid)], _PROFILE) + + assert elem_tree.tostring(root) == before + + +def test_bad_later_identifier_addition_does_not_partially_mutate_base(tmp_path): + first = _write_additions( + tmp_path, + _instruction('VALID', opcode=3), + identifier='first', + filename='first.xml', + identifier_additions=_identifier_addition(3), + ) + second = _write_additions( + tmp_path, + identifier='second', + filename='second.xml', + identifier_additions=_identifier_addition(4), + ) + root = _base_root() + before = elem_tree.tostring(root) + + with pytest.raises(IsaAdditionError, match=r'opcode 4 is unowned'): + apply_isa_additions(root, [str(first), str(second)], _PROFILE) + + assert elem_tree.tostring(root) == before + + +@pytest.mark.parametrize( + ('keyword', 'value', 'message'), + [ + ('architecture', 'AMD OTHER 1', 'base architecture'), + ('schema', '1.1.1', 'base schema'), + ('identifier', 'bad id', 'invalid additions document Id'), + ], +) +def test_additions_metadata_must_match_base(tmp_path, keyword, value, message): + addition = _write_additions(tmp_path, _instruction(), **{keyword: value}) + + with pytest.raises(IsaAdditionError, match=message): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +def test_duplicate_additions_document_id_is_rejected(tmp_path): + first = _write_additions(tmp_path, identifier='same', filename='first.xml') + second = _write_additions(tmp_path, identifier='same', filename='second.xml') + + with pytest.raises(IsaAdditionError, match='duplicate additions document Id'): + apply_isa_additions(_base_root(), [str(first), str(second)], _PROFILE) + + +def test_duplicate_instruction_across_additions_is_rejected(tmp_path): + first = _write_additions( + tmp_path, + _instruction('SAME_NAME', opcode=2), + identifier='first', + filename='first.xml', + ) + second = _write_additions( + tmp_path, + _instruction('SAME_NAME', opcode=3), + identifier='second', + filename='second.xml', + ) + + with pytest.raises(IsaAdditionError, match='already exists'): + apply_isa_additions(_base_root(), [str(first), str(second)], _PROFILE) + + +def test_same_instruction_may_repeat_slot_for_encoding_conditions(tmp_path): + instructions = _instruction().replace( + ' ', + '''\ + + ENC_TEST + alternate + 2 + + OPR_TEST + + + ''', + ) + addition = _write_additions(tmp_path, instructions) + root = _base_root() + + apply_isa_additions(root, [str(addition)], _PROFILE) + + added = root.findall('./ISA/Instructions/Instruction')[-1] + assert len(added.findall('./InstructionEncodings/InstructionEncoding')) == 2 + + +def test_same_instruction_cannot_repeat_slot_for_same_encoding_condition(tmp_path): + instruction = elem_tree.fromstring(_instruction()) + encodings = instruction.find('InstructionEncodings') + encodings.append(deepcopy(encodings[0])) + addition = _write_additions( + tmp_path, elem_tree.tostring(instruction, encoding='unicode') + ) + + with pytest.raises(IsaAdditionError, match='repeats opcode.*condition'): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +@pytest.mark.parametrize( + ('instructions', 'message'), + [ + (_instruction('BASE_INST', opcode=2), 'already exists'), + (_instruction('ADDITION_INST', opcode=1), 'already owned'), + (_instruction(encoding='ENC_MISSING'), 'unknown encoding'), + (_instruction(operand_type='OPR_MISSING'), 'unknown operand type'), + (_instruction(opcode='not-decimal'), 'invalid decimal opcode'), + (_instruction(opcode=-1), 'negative opcode'), + (_instruction(opcode=8), 'outside the encoding range'), + ], +) +def test_conflicting_or_invalid_additions_are_rejected(tmp_path, instructions, message): + addition = _write_additions(tmp_path, instructions) + + with pytest.raises(IsaAdditionError, match=message): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +@pytest.mark.parametrize( + ('identifier_additions', 'instructions', 'message'), + [ + ( + _identifier_addition(3, encoding='ENC_MISSING'), + _instruction(opcode=3), + 'unknown encoding', + ), + ( + _identifier_addition(3, text=f'{_identifier_text(3)[:-1]}x'), + _instruction(opcode=3), + 'malformed radix-2 identifier', + ), + ( + _identifier_addition(3, text=_identifier_text(3)[:-1]), + _instruction(opcode=3), + 'identifier width 31.*encoding width 32', + ), + ( + _identifier_addition(3, radix=16), + _instruction(opcode=3), + 'identifier radix 16.*encoding radix 2', + ), + ( + _identifier_addition(3, text=f'0{_identifier_text(3)[1:]}'), + _instruction(opcode=3), + 'incompatible with the base identifier mask/layout', + ), + ( + _identifier_addition(4, declared_opcode=3), + _instruction(opcode=3), + 'decodes opcode 4, not declared opcode 3', + ), + (_identifier_addition(3), '', 'opcode 3 is unowned'), + (_identifier_addition(1), '', 'duplicate identifier'), + ], +) +def test_invalid_identifier_additions_are_rejected( + tmp_path, identifier_additions, instructions, message +): + addition = _write_additions( + tmp_path, + instructions, + identifier_additions=identifier_additions, + ) + + with pytest.raises(IsaAdditionError, match=message): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +@pytest.mark.parametrize( + ('fragment', 'message'), + [ + ( + '3' + f'{_identifier_text(3)}' + '', + 'exactly one ', + ), + ( + 'ENC_TEST' + f'{_identifier_text(3)}' + '', + 'exactly one ', + ), + ( + 'ENC_TEST' + '3', + 'exactly one ', + ), + ( + _identifier_addition(3).replace('Radix="2"', 'Radix="2" Replace="true"'), + 'unknown attributes: Replace', + ), + ], +) +def test_missing_or_unknown_identifier_structure_is_rejected( + tmp_path, fragment, message +): + addition = _write_additions( + tmp_path, + _instruction(opcode=3), + identifier_additions=fragment, + ) + + with pytest.raises(IsaAdditionError, match=message): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +def test_identifier_collision_against_earlier_addition_is_rejected(tmp_path): + first = _write_additions( + tmp_path, + _instruction(opcode=3), + identifier='first', + filename='first.xml', + identifier_additions=_identifier_addition(3), + ) + second = _write_additions( + tmp_path, + identifier='second', + filename='second.xml', + identifier_additions=_identifier_addition( + 3, text=_identifier_text(3, alternate_layout=True) + ), + ) + + with pytest.raises(IsaAdditionError, match=r'collides.*opcode 3'): + apply_isa_additions(_base_root(), [str(first), str(second)], _PROFILE) + + +def test_identifier_opcode_collision_with_different_encoding_value_is_rejected( + tmp_path, +): + first = _write_additions( + tmp_path, + _instruction(opcode=3), + identifier='first', + filename='first.xml', + identifier_additions=_identifier_addition(3), + ) + second = _write_additions( + tmp_path, + identifier='second', + filename='second.xml', + identifier_additions=_identifier_addition( + 3, + text=_identifier_text(3, alternate_encoding_value=True), + ), + ) + + with pytest.raises(IsaAdditionError, match=r'collides.*opcode 3'): + apply_isa_additions(_base_root(), [str(first), str(second)], _PROFILE) + + +def test_identifier_collision_against_base_is_rejected(tmp_path): + addition = _write_additions( + tmp_path, + identifier_additions=_identifier_addition( + 1, text=_identifier_text(1, alternate_layout=True) + ), + ) + + with pytest.raises(IsaAdditionError, match=r'collides.*opcode 1'): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +def test_duplicate_identifier_against_earlier_addition_is_rejected(tmp_path): + first = _write_additions( + tmp_path, + _instruction(opcode=3), + identifier='first', + filename='first.xml', + identifier_additions=_identifier_addition(3), + ) + second = _write_additions( + tmp_path, + identifier='second', + filename='second.xml', + identifier_additions=_identifier_addition(3), + ) + + with pytest.raises(IsaAdditionError, match='duplicate identifier'): + apply_isa_additions(_base_root(), [str(first), str(second)], _PROFILE) + + +def test_empty_identifier_additions_group_is_rejected(tmp_path): + addition = _write_additions(tmp_path, identifier_additions='') + + with pytest.raises(IsaAdditionError, match='must not be empty'): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +def test_unknown_additions_structure_is_rejected(tmp_path): + addition = _write_additions(tmp_path, extra_root='') + + with pytest.raises(IsaAdditionError, match='unknown additions root elements'): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +def test_unknown_additions_attribute_is_rejected(tmp_path): + addition = _write_additions(tmp_path, extra_attributes=' Replace="true"') + + with pytest.raises(IsaAdditionError, match='unknown additions root attributes'): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +@pytest.mark.parametrize( + ('container', 'kwargs'), + [ + ('InstructionAdditions', {'instructions_attributes': ' Replace="true"'}), + ( + 'EncodingIdentifierAdditions', + { + 'identifier_additions': _identifier_addition(3), + 'identifier_container_attributes': ' Replace="true"', + }, + ), + ], +) +def test_additions_container_attributes_are_rejected(tmp_path, container, kwargs): + addition = _write_additions(tmp_path, **kwargs) + + with pytest.raises( + IsaAdditionError, match=rf'<{container}> must not have attributes: Replace' + ): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +def test_cli_additions_arguments_are_grouped_in_order(): + assert _group_isa_additions_args( + ['cdna5:first.xml', 'rdna4:other.xml', 'cdna5:second.xml'] + ) == { + 'cdna5': ['first.xml', 'second.xml'], + 'rdna4': ['other.xml'], + } + + +@pytest.mark.parametrize('entry', ['missing-colon', ':additions.xml', 'cdna5:']) +def test_cli_additions_argument_requires_name_and_path(entry): + with pytest.raises(ValueError, match='name'): + _group_isa_additions_args([entry]) + + +def _cdna5_sop1_identifier(opcode: int, *, implied_literal: bool = False) -> str: + if implied_literal: + value = list( + '00000000000000000000000000000000' '10111110100000000000000000000000' + ) + value[48:56] = f'{opcode:08b}' + else: + value = list('10111110100000000000000000000000') + value[16:24] = f'{opcode:08b}' + return ''.join(value) + + +_CDNA5_SOP1_OPCODE7_BYTES = bytes.fromhex('00 07 80 be') +_CDNA5_SOP1_LITERAL_OPCODE7_BYTES = bytes.fromhex('00 07 80 be 00 00 00 00') + + +def _cdna5_primary_table_index(encoded: bytes) -> int: + return int.from_bytes(encoded[:4], byteorder='little') >> 23 + + +def _assert_sop1_opcode7_decode_mapping(spec) -> None: + table_index = _cdna5_primary_table_index(_CDNA5_SOP1_OPCODE7_BYTES) + assert table_index == 381 + assert _cdna5_sop1_identifier(7) == ( + f'{int.from_bytes(_CDNA5_SOP1_OPCODE7_BYTES, byteorder="little"):032b}' + ) + assert spec.encoding_map['ENC_SOP1'].primary_dt_ptrs[7] == table_index + + entry = spec.primary_decode_table[table_index] + assert entry.decode_func == 'subDecodeSop1' + assert entry.sub_decode_table == 'sub_decode_sop1' + assert entry.sub_decode_funcs[7] == 'decodeSAdditionTestB32Sop1' + + +def _write_cdna5_clone_additions( + tmp_path: Path, + base_xml: Path, + *, + opcode: int = 7, + add_identifiers: bool = False, + include_implied_literal: bool = False, +) -> Path: + base_root = elem_tree.parse(base_xml).getroot() + original = next( + instruction + for instruction in base_root.findall('./ISA/Instructions/Instruction') + if instruction.findtext('InstructionName') == 'S_MOV_B32' + ) + addition = deepcopy(original) + addition.find('InstructionName').text = 'S_ADDITION_TEST_B32' + retained_encodings = {'ENC_SOP1'} + if include_implied_literal: + retained_encodings.add('SOP1_INST_LITERAL') + instruction_encodings = addition.find('InstructionEncodings') + for encoding in list(instruction_encodings): + if encoding.findtext('EncodingName') not in retained_encodings: + instruction_encodings.remove(encoding) + for opcode_node in addition.findall( + './InstructionEncodings/InstructionEncoding/Opcode' + ): + opcode_node.text = str(opcode) + + addition_root = elem_tree.Element( + 'IsaAdditions', + { + 'Id': 'cdna5-test', + 'BaseArchitecture': 'AMD CDNA 5', + 'BaseSchemaVersion': '1.2.0', + }, + ) + if add_identifiers: + identifier_additions = elem_tree.SubElement( + addition_root, 'EncodingIdentifierAdditions' + ) + for encoding_name, implied_literal in ( + ('ENC_SOP1', False), + ('SOP1_INST_LITERAL', True), + ): + if implied_literal and not include_implied_literal: + continue + identifier_addition = elem_tree.SubElement( + identifier_additions, 'EncodingIdentifierAddition' + ) + elem_tree.SubElement(identifier_addition, 'EncodingName').text = ( + encoding_name + ) + elem_tree.SubElement(identifier_addition, 'Opcode').text = str(opcode) + identifier = elem_tree.SubElement( + identifier_addition, 'EncodingIdentifier', {'Radix': '2'} + ) + identifier.text = _cdna5_sop1_identifier( + opcode, implied_literal=implied_literal + ) + instructions = elem_tree.SubElement(addition_root, 'InstructionAdditions') + instructions.append(addition) + path = tmp_path / 'cdna5-test.xml' + elem_tree.ElementTree(addition_root).write(path, encoding='unicode') + return path + + +def test_parser_retains_additions_provenance_on_added_instructions(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions(tmp_path, base_xml, add_identifiers=True) + + spec = Parser(str(base_xml), Cdna5Profile(), [str(addition)]).parse() + + assert [(item.identifier, item.path) for item in spec.applied_additions] == [ + ('cdna5-test', str(addition)) + ] + added = [ + instruction + for encoding in spec.inst_encodings + for instruction in encoding.insts + if instruction.name == 'S_ADDITION_TEST_B32' + ] + assert added + assert all( + instruction.source_addition == spec.applied_additions[0] + for instruction in added + ) + _assert_sop1_opcode7_decode_mapping(spec) + + +def test_primary_and_implied_literal_identifiers_build_decode_pointers(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions( + tmp_path, + base_xml, + add_identifiers=True, + include_implied_literal=True, + ) + + spec = Parser(str(base_xml), Cdna5Profile(), [str(addition)]).parse() + + _assert_sop1_opcode7_decode_mapping(spec) + literal_table_index = _cdna5_primary_table_index(_CDNA5_SOP1_LITERAL_OPCODE7_BYTES) + assert literal_table_index == 381 + assert _cdna5_sop1_identifier(7, implied_literal=True) == ( + f'{int.from_bytes(_CDNA5_SOP1_LITERAL_OPCODE7_BYTES, byteorder="little"):064b}' + ) + assert ( + spec.encoding_map['SOP1_INST_LITERAL'].primary_dt_ptrs[7] == literal_table_index + ) + literal_entry = spec.primary_decode_table[literal_table_index] + assert literal_entry.decode_func == 'subDecodeSop1' + assert literal_entry.sub_decode_table == 'sub_decode_sop1' + assert literal_entry.sub_decode_funcs[7] == 'decodeSAdditionTestB32Sop1' + + +def test_implied_literal_identifier_requires_matching_parent_owner(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions( + tmp_path, + base_xml, + add_identifiers=True, + include_implied_literal=True, + ) + tree = elem_tree.parse(addition) + for parent_path in ( + './EncodingIdentifierAdditions', + './InstructionAdditions/Instruction/InstructionEncodings', + ): + parent = tree.find(parent_path) + for child in list(parent): + if child.findtext('EncodingName') == 'ENC_SOP1': + parent.remove(child) + tree.write(addition, encoding='unicode') + + with pytest.raises( + IsaAdditionError, match=r'implied-literal identifier.*parent encoding.*owner' + ): + Parser(str(base_xml), Cdna5Profile(), [str(addition)]).parse() + + +def test_parser_rejects_added_instruction_without_decode_identifier(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions(tmp_path, base_xml) + + with pytest.raises( + IsaAdditionError, match=r'S_ADDITION_TEST_B32.*opcode 7.*unreachable' + ): + Parser(str(base_xml), Cdna5Profile(), [str(addition)]).parse() + + +def test_later_parse_without_additions_is_not_affected(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions(tmp_path, base_xml, add_identifiers=True) + + with_addition = Parser(str(base_xml), Cdna5Profile(), [str(addition)]).parse() + without_addition = Parser(str(base_xml), Cdna5Profile()).parse() + + assert with_addition.encoding_map['ENC_SOP1'].primary_dt_ptrs[7] != -1 + assert without_addition.encoding_map['ENC_SOP1'].primary_dt_ptrs[7] == -1 + assert all( + instruction.name != 'S_ADDITION_TEST_B32' + for encoding in without_addition.inst_encodings + for instruction in encoding.insts + ) + + +def test_parser_rejects_added_opcode_outside_base_encoding_range(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions(tmp_path, base_xml) + tree = elem_tree.parse(addition) + for opcode in tree.findall( + './InstructionAdditions/Instruction/InstructionEncodings/' + 'InstructionEncoding/Opcode' + ): + opcode.text = '999999' + tree.write(addition, encoding='unicode') + + with pytest.raises( + IsaAdditionError, + match=r'S_ADDITION_TEST_B32.*/ENC_SOP1 opcode 999999.*outside the encoding range', + ): + Parser(str(base_xml), Cdna5Profile(), [str(addition)]).parse() + + +def _generated_files(root: Path) -> dict[Path, bytes]: + return { + path.relative_to(root): path.read_bytes() + for path in root.rglob('*') + if path.is_file() + } + + +def test_empty_additions_document_is_byte_identical_through_cli(tmp_path): + repo_root = Path(__file__).resolve().parents[6] + base_xml = ( + repo_root / 'shared' / 'machine-readable-isa' / 'isa' / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_additions( + tmp_path, + identifier='empty-cdna5', + architecture='AMD CDNA 5', + filename='empty-cdna5.xml', + ) + baseline_output = tmp_path / 'baseline' + additions_output = tmp_path / 'with-additions' + baseline_output.mkdir() + additions_output.mkdir() + + subprocess.run( + [ + sys.executable, + '-m', + 'amdisa', + f'cdna5:{base_xml}', + '--isa-output', + str(baseline_output / 'isa'), + '--include-root', + str(baseline_output), + '--dbt-output', + str(baseline_output / 'dbt'), + ], + check=True, + capture_output=True, + text=True, + ) + subprocess.run( + [ + sys.executable, + '-m', + 'amdisa', + f'cdna5:{base_xml}', + '--isa-additions', + f'cdna5:{addition}', + '--isa-output', + str(additions_output / 'isa'), + '--include-root', + str(additions_output), + '--dbt-output', + str(additions_output / 'dbt'), + ], + check=True, + capture_output=True, + text=True, + ) + + assert _generated_files(additions_output) == _generated_files(baseline_output) + + +def test_nonempty_additions_document_generates_decoder_dispatch(tmp_path): + repo_root = Path(__file__).resolve().parents[6] + base_xml = ( + repo_root / 'shared' / 'machine-readable-isa' / 'isa' / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions(tmp_path, base_xml, add_identifiers=True) + output = tmp_path / 'generated' + output.mkdir() + + subprocess.run( + [ + sys.executable, + '-m', + 'amdisa', + f'cdna5:{base_xml}', + '--isa-additions', + f'cdna5:{addition}', + '--isa-output', + str(output / 'isa'), + '--dbt-output', + str(output / 'dbt'), + ], + check=True, + capture_output=True, + text=True, + ) + + decoder = (output / 'isa' / 'cdna5' / 'decoder.cpp').read_text() + match = re.search( + r'DecoderImpl::sub_decode_sop1 = \{\s*(.*?)\s*\};', decoder, re.DOTALL + ) + assert match is not None + entries = [entry.strip() for entry in match.group(1).split(',')] + assert entries[7] == '&detail::decodeSAdditionTestB32Sop1' From f6135a20aa778c58c9aedb62f9ced485d7141c80 Mon Sep 17 00:00:00 2001 From: hanhanW Date: Wed, 26 Aug 2026 16:01:03 -0700 Subject: [PATCH 2/5] [rocjitsu] Harden ISA additions validation ISA additions could pass validation and fail later in parsing or code generation when an instruction node was incomplete, occupied a synthetic compatibility slot, or normalized to an existing C++ symbol. Modifier-bearing encodings could also reject valid identifiers because ownership was checked against the decoded base opcode. Validate complete MR-ISA instruction subtrees before merging them. Reserve compatibility instruction slots in ISA profiles, share generated-name formatting between validation and codegen, and use modifier-expanded opcodes for both ownership and reachability. Add focused regressions for malformed nodes, CDNA5 and RDNA4 slot collisions, CDNA1 MIMG OPM routing, and normalized-name collisions. --- .../lib/python/amdisa/codegen/_generator.py | 15 +- .../rocjitsu/lib/python/amdisa/gpuisa.py | 71 ++- .../lib/python/amdisa/isa_additions.py | 427 +++++++++++++++++- .../rocjitsu/lib/python/amdisa/isa_profile.py | 13 + .../python/amdisa/tests/test_isa_additions.py | 300 +++++++++++- 5 files changed, 775 insertions(+), 51 deletions(-) diff --git a/emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py b/emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py index bde9d14ade62..1e07d579e29d 100644 --- a/emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py +++ b/emulation/rocjitsu/lib/python/amdisa/codegen/_generator.py @@ -36,6 +36,8 @@ IsaSpec, Operand, OperandNamePattern, + opcode_constant_base_name, + opcode_name_fragment, ) from amdisa.fieldless_policy import ( FieldlessCategory, @@ -475,20 +477,11 @@ def _opcode_name_fragment(token: str) -> str: meant to be read and used directly by handwritten DBT code, so split a few packed ISA abbreviations that commonly appear inside one XML token. """ - token = token.lower() - if token.endswith('exec') and len(token) > len('exec'): - return f'{token[:-4].capitalize()}Exec' - if token.endswith('pc') and len(token) > len('pc'): - return f'{token[:-2].capitalize()}Pc' - return token.capitalize() + return opcode_name_fragment(token) @classmethod def _opcode_const_base_name(cls, mnemonic: str) -> str: - return 'k' + ''.join( - cls._opcode_name_fragment(token) - for token in mnemonic.lower().split('_') - if token - ) + return opcode_constant_base_name(mnemonic) @staticmethod def _emit_opcode_constant(name: str, value: int) -> str: diff --git a/emulation/rocjitsu/lib/python/amdisa/gpuisa.py b/emulation/rocjitsu/lib/python/amdisa/gpuisa.py index 9d3f56a1da7c..1abe82ad8920 100644 --- a/emulation/rocjitsu/lib/python/amdisa/gpuisa.py +++ b/emulation/rocjitsu/lib/python/amdisa/gpuisa.py @@ -50,6 +50,60 @@ class MicrocodeField: } +def format_encoding_name(enc_name: str) -> str: + """Format an MR-ISA encoding name as a C++ PascalCase fragment.""" + parts = enc_name.split('_') + if parts[0] == 'ENC': + parts = parts[1:] + return ''.join(part.capitalize() for part in parts) + + +def format_true_encoding_name( + enc_name: str, *, is_implied_literal_enc: bool = False +) -> str: + """Format the concrete C++ encoding class used by an instruction.""" + if is_implied_literal_enc: + return enc_name.split('_')[0].capitalize() + return format_encoding_name(enc_name) + + +def format_instruction_name( + name: str, enc_name: str, *, is_implied_literal_enc: bool = False +) -> str: + """Format an instruction's generated C++ class/decode-function name.""" + mnemonic = ''.join(part.capitalize() for part in name.split('_')) + encoding = format_true_encoding_name( + enc_name, is_implied_literal_enc=is_implied_literal_enc + ) + return f'{mnemonic}{encoding}' + + +def opcode_name_fragment(token: str) -> str: + """Return one C++ opcode-constant fragment for a mnemonic token.""" + token = token.lower() + if token.endswith('exec') and len(token) > len('exec'): + return f'{token[:-4].capitalize()}Exec' + if token.endswith('pc') and len(token) > len('pc'): + return f'{token[:-2].capitalize()}Pc' + return token.capitalize() + + +def opcode_constant_base_name(mnemonic: str) -> str: + """Return the generated C++ opcode-constant base for a mnemonic.""" + return 'k' + ''.join( + opcode_name_fragment(token) for token in mnemonic.lower().split('_') if token + ) + + +def opcode_constant_name( + mnemonic: str, enc_name: str, *, is_implied_literal_enc: bool = False +) -> str: + """Return the concrete encoding-qualified C++ opcode-constant name.""" + return opcode_constant_base_name(mnemonic) + format_true_encoding_name( + enc_name, is_implied_literal_enc=is_implied_literal_enc + ) + + def synthesize_fieldless_name(operand_type: str) -> str: """Return a stable, valid C++ base identifier for a fieldless operand. @@ -162,9 +216,7 @@ def __init__(self, enc_name: str, is_implied_literal_enc: bool = False) -> None: @cached_property def fmt_enc_name(self) -> str: """Encoding name formatted to C++ PascalCase style.""" - if self.enc_name.split('_')[0] == 'ENC': - return ''.join(x.capitalize() for x in self.enc_name.split('_')[1:]) - return ''.join(x.capitalize() for x in self.enc_name.split('_')) + return format_encoding_name(self.enc_name) @cached_property def fmt_true_enc_name(self) -> str: @@ -174,9 +226,9 @@ def fmt_true_enc_name(self) -> str: their parent encoding name (e.g., ``Vop2``) because the C++ class hierarchy inherits from the parent encoding class. """ - if self.is_implied_literal_enc: - return self.enc_name.split('_')[0].capitalize() - return self.fmt_enc_name + return format_true_encoding_name( + self.enc_name, is_implied_literal_enc=self.is_implied_literal_enc + ) class InstEncoding(InstBase): @@ -267,9 +319,10 @@ def __init__( @cached_property def fmt_name(self) -> str: """Instruction name formatted to C++ PascalCase style.""" - return ( - f'{"".join(x.capitalize() for x in self.name.split("_"))}' - f'{self.fmt_true_enc_name}' + return format_instruction_name( + self.name, + self.enc_name, + is_implied_literal_enc=self.is_implied_literal_enc, ) @cached_property diff --git a/emulation/rocjitsu/lib/python/amdisa/isa_additions.py b/emulation/rocjitsu/lib/python/amdisa/isa_additions.py index 50068a4e78b7..8dacd70d7fa4 100644 --- a/emulation/rocjitsu/lib/python/amdisa/isa_additions.py +++ b/emulation/rocjitsu/lib/python/amdisa/isa_additions.py @@ -17,7 +17,11 @@ import xml.etree.ElementTree as elem_tree from amdisa import xml_schema as xs -from amdisa.gpuisa import IsaAdditionProvenance +from amdisa.gpuisa import ( + IsaAdditionProvenance, + format_instruction_name, + opcode_constant_name, +) from amdisa.isa_profile import IsaProfile ADDITIONS_ROOT = 'IsaAdditions' @@ -133,6 +137,255 @@ def _required_decimal(text: str, context: str) -> int: return value +def _validate_node_shape( + node: elem_tree.Element, + context: str, + *, + allowed_attributes: frozenset[str] = frozenset(), + required_attributes: frozenset[str] = frozenset(), + child_counts: Mapping[str, tuple[int, int | None]] | None = None, +) -> None: + unknown_attributes = set(node.attrib) - allowed_attributes + if unknown_attributes: + names = ', '.join(sorted(unknown_attributes)) + raise IsaAdditionError(f'{context}: unknown attributes: {names}') + missing_attributes = required_attributes - set(node.attrib) + if missing_attributes: + names = ', '.join(sorted(missing_attributes)) + raise IsaAdditionError(f'{context}: missing required attributes: {names}') + + allowed_children = child_counts or {} + unknown_children = [ + child.tag for child in node if child.tag not in allowed_children + ] + if unknown_children: + names = ', '.join(f'<{name}>' for name in unknown_children) + raise IsaAdditionError(f'{context}: unknown elements: {names}') + for tag, (minimum, maximum) in allowed_children.items(): + count = sum(child.tag == tag for child in node) + if count < minimum or (maximum is not None and count > maximum): + if minimum == maximum: + expected = f'exactly {minimum}' + elif maximum is None: + expected = f'at least {minimum}' + else: + expected = f'between {minimum} and {maximum}' + raise IsaAdditionError( + f'{context}: expected {expected} <{tag}> element(s), found {count}' + ) + + +def _validate_text_leaf( + node: elem_tree.Element, + context: str, + *, + allowed_attributes: frozenset[str] = frozenset(), + required_attributes: frozenset[str] = frozenset(), +) -> str: + _validate_node_shape( + node, + context, + allowed_attributes=allowed_attributes, + required_attributes=required_attributes, + ) + if node.text is None or not node.text.strip(): + raise IsaAdditionError(f'{context}: value must not be empty') + return node.text.strip() + + +def _validate_boolean_text(node: elem_tree.Element, context: str) -> None: + value = _validate_text_leaf(node, context).lower() + if value not in {'true', 'false'}: + raise IsaAdditionError( + f'{context}: expected boolean true or false, found {value!r}' + ) + + +def _validate_instruction_structure( + instruction: elem_tree.Element, context: str +) -> None: + """Validate one complete Instruction subtree accepted by Parser.parse_insts.""" + _validate_node_shape( + instruction, + context, + child_counts={ + xs.INST_FLAGS: (1, 1), + xs.INST_NAME: (1, 1), + 'AliasedInstructionNames': (0, 1), + 'Description': (1, 1), + xs.INST_ENCODINGS: (1, 1), + 'FunctionalGroup': (1, 1), + }, + ) + + flags = instruction.find(xs.INST_FLAGS) + assert flags is not None + flag_names = ( + 'IsBranch', + 'IsConditionalBranch', + 'IsIndirectBranch', + 'IsProgramTerminator', + 'IsImmediatelyExecuted', + ) + _validate_node_shape( + flags, + f'{context}/<{xs.INST_FLAGS}>', + child_counts={name: (1, 1) for name in flag_names}, + ) + for name in flag_names: + node = flags.find(name) + assert node is not None + _validate_boolean_text(node, f'{context}/<{xs.INST_FLAGS}>/<{name}>') + + name_node = instruction.find(xs.INST_NAME) + description = instruction.find('Description') + assert name_node is not None and description is not None + _validate_text_leaf(name_node, f'{context}/<{xs.INST_NAME}>') + _validate_text_leaf(description, f'{context}/') + + aliases = instruction.find('AliasedInstructionNames') + if aliases is not None: + _validate_node_shape( + aliases, + f'{context}/', + child_counts={xs.INST_NAME: (1, None)}, + ) + for alias in aliases: + _validate_text_leaf( + alias, f'{context}//<{xs.INST_NAME}>' + ) + + encodings = instruction.find(xs.INST_ENCODINGS) + assert encodings is not None + _validate_node_shape( + encodings, + f'{context}/<{xs.INST_ENCODINGS}>', + child_counts={xs.INST_ENCODING: (1, None)}, + ) + for encoding in encodings: + encoding_context = f'{context}/<{xs.INST_ENCODING}>' + _validate_node_shape( + encoding, + encoding_context, + child_counts={ + xs.ENCODING_NAME: (1, 1), + xs.ENCODING_COND: (1, 1), + xs.OPCODE: (1, 1), + xs.OPERANDS: (1, 1), + }, + ) + encoding_name = encoding.find(xs.ENCODING_NAME) + condition = encoding.find(xs.ENCODING_COND) + opcode = encoding.find(xs.OPCODE) + operands = encoding.find(xs.OPERANDS) + assert ( + encoding_name is not None + and condition is not None + and opcode is not None + and operands is not None + ) + _validate_text_leaf(encoding_name, f'{encoding_context}/<{xs.ENCODING_NAME}>') + _validate_text_leaf( + condition, + f'{encoding_context}/<{xs.ENCODING_COND}>', + allowed_attributes=frozenset({'Id'}), + ) + _validate_text_leaf( + opcode, + f'{encoding_context}/<{xs.OPCODE}>', + allowed_attributes=frozenset({xs.ENC_IDENTIFER_ATTR_RADIX}), + required_attributes=frozenset({xs.ENC_IDENTIFER_ATTR_RADIX}), + ) + if opcode.attrib[xs.ENC_IDENTIFER_ATTR_RADIX] != '10': + raise IsaAdditionError( + f'{encoding_context}/<{xs.OPCODE}>: Radix must be 10' + ) + _validate_node_shape( + operands, + f'{encoding_context}/<{xs.OPERANDS}>', + child_counts={xs.OPERAND: (0, None)}, + ) + for operand in operands: + operand_context = f'{encoding_context}/<{xs.OPERANDS}>/<{xs.OPERAND}>' + operand_attributes = frozenset( + { + xs.OPERAND_ATTR_INPUT, + xs.OPERAND_ATTR_OUTPUT, + xs.OPERAND_ATTR_IS_IMPLICIT, + xs.OPERAND_ATTR_IS_BINARY_MICROCODE_REQUIRED, + xs.OPERAND_ATTR_ORDER, + } + ) + _validate_node_shape( + operand, + operand_context, + allowed_attributes=operand_attributes, + required_attributes=operand_attributes, + child_counts={ + xs.FIELD_NAME: (0, 1), + xs.DATA_FORMAT_NAME: (1, 1), + xs.OPERAND_TYPE: (1, 1), + xs.OPERAND_SIZE: (1, 1), + }, + ) + for attribute in ( + xs.OPERAND_ATTR_INPUT, + xs.OPERAND_ATTR_OUTPUT, + xs.OPERAND_ATTR_IS_IMPLICIT, + xs.OPERAND_ATTR_IS_BINARY_MICROCODE_REQUIRED, + ): + value = operand.attrib[attribute].lower() + if value not in {'true', 'false'}: + raise IsaAdditionError( + f'{operand_context}: attribute {attribute} must be ' + f'true or false, found {value!r}' + ) + order = _required_decimal( + operand.attrib[xs.OPERAND_ATTR_ORDER], + f'{operand_context} Order', + ) + if order < 1: + raise IsaAdditionError(f'{operand_context}: Order must be positive') + for tag in (xs.FIELD_NAME, xs.DATA_FORMAT_NAME, xs.OPERAND_TYPE): + node = operand.find(tag) + if node is not None: + _validate_text_leaf(node, f'{operand_context}/<{tag}>') + size_node = operand.find(xs.OPERAND_SIZE) + assert size_node is not None + size = _required_decimal( + _validate_text_leaf( + size_node, f'{operand_context}/<{xs.OPERAND_SIZE}>' + ), + f'{operand_context}/<{xs.OPERAND_SIZE}>', + ) + if size < 1: + raise IsaAdditionError( + f'{operand_context}/<{xs.OPERAND_SIZE}>: value must be positive' + ) + + functional_group = instruction.find('FunctionalGroup') + assert functional_group is not None + functional_context = f'{context}/' + _validate_node_shape( + functional_group, + functional_context, + child_counts={'Name': (1, 1), 'FunctionalSubgroups': (1, 1)}, + ) + functional_name = functional_group.find('Name') + subgroups = functional_group.find('FunctionalSubgroups') + assert functional_name is not None and subgroups is not None + _validate_text_leaf(functional_name, f'{functional_context}/') + _validate_node_shape( + subgroups, + f'{functional_context}/', + child_counts={'Subgroup': (1, None)}, + ) + for subgroup in subgroups: + _validate_text_leaf( + subgroup, f'{functional_context}//' + ) + + def _binary_text(node: elem_tree.Element, bit_width: int, context: str) -> str: unknown_attributes = set(node.attrib) - {xs.ENC_IDENTIFER_ATTR_RADIX} if unknown_attributes: @@ -236,6 +489,8 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ dict[tuple[str, int], str], set[str], set[str], + dict[str, str], + dict[str, str], ]: isa_node = xs.get_node(root, xs.ISA) encodings_node = xs.get_node(isa_node, xs.ENCODINGS) @@ -344,6 +599,8 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ } instruction_names: set[str] = set() opcode_owners: dict[tuple[str, int], str] = {} + instruction_symbol_owners: dict[str, str] = {} + opcode_symbol_owners: dict[str, str] = {} for instruction in instructions_node: name = _required_text(instruction, xs.INST_NAME, 'base instruction') instruction_names.add(name) @@ -365,12 +622,66 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ f'{opcode_text!r}' ) from error opcode_owners.setdefault((enc_name, opcode), name) + encoding_definition = encoding_definitions.get(enc_name) + condition = _required_text( + encoding, + xs.ENCODING_COND, + f'base instruction {name}/{enc_name}', + ) + if encoding_definition is None or profile.skip_inst_encoding( + enc_name, condition + ): + continue + is_implied_literal = encoding_definition.is_implied_literal + instruction_symbol_owners.setdefault( + format_instruction_name( + name, + enc_name, + is_implied_literal_enc=is_implied_literal, + ), + name, + ) + opcode_symbol_owners.setdefault( + opcode_constant_name( + name, + enc_name, + is_implied_literal_enc=is_implied_literal, + ), + name, + ) + + for slot, owner in profile.compatibility_instruction_slots.items(): + enc_name, _ = slot + encoding_definition = encoding_definitions.get(enc_name) + if encoding_definition is None: + continue + opcode_owners.setdefault(slot, owner) + instruction_names.add(owner) + is_implied_literal = encoding_definition.is_implied_literal + instruction_symbol_owners.setdefault( + format_instruction_name( + owner, + enc_name, + is_implied_literal_enc=is_implied_literal, + ), + owner, + ) + opcode_symbol_owners.setdefault( + opcode_constant_name( + owner, + enc_name, + is_implied_literal_enc=is_implied_literal, + ), + owner, + ) return ( encoding_definitions, encoding_names, opcode_owners, operand_types, instruction_names, + instruction_symbol_owners, + opcode_symbol_owners, ) @@ -463,11 +774,15 @@ def _validate_instruction_addition( *, path: str, addition_id: str, + encodings: Mapping[str, _EncodingDefinition], encoding_names: set[str], opcode_owners: dict[tuple[str, int], str], opcode_limits: Mapping[str, int], operand_types: set[str], instruction_names: set[str], + instruction_symbol_owners: dict[str, str], + opcode_symbol_owners: dict[str, str], + profile: IsaProfile, ) -> _InstructionAddition: if instruction.tag != xs.INST: raise IsaAdditionError( @@ -476,6 +791,7 @@ def _validate_instruction_addition( ) context = f'{path}: additions document {addition_id!r}' + _validate_instruction_structure(instruction, f'{context}: <{xs.INST}>') name = _required_text(instruction, xs.INST_NAME, context) if name in instruction_names: raise IsaAdditionError(f'{context}: instruction {name!r} already exists') @@ -545,6 +861,41 @@ def _validate_instruction_addition( forms.add(form) forms_in_order.append(form) + encoding_definition = encodings.get(enc_name) + if encoding_definition is not None and not profile.skip_inst_encoding( + enc_name, condition + ): + is_implied_literal = encoding_definition.is_implied_literal + generated_symbols = ( + ( + 'instruction', + format_instruction_name( + name, + enc_name, + is_implied_literal_enc=is_implied_literal, + ), + instruction_symbol_owners, + ), + ( + 'opcode constant', + opcode_constant_name( + name, + enc_name, + is_implied_literal_enc=is_implied_literal, + ), + opcode_symbol_owners, + ), + ) + for symbol_kind, symbol, owners in generated_symbols: + symbol_owner = owners.get(symbol) + if symbol_owner is not None and symbol_owner != name: + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} normalizes ' + f'to {symbol_kind} symbol {symbol!r}, already owned by ' + f'instruction {symbol_owner!r}' + ) + owners[symbol] = name + operands = encoding.find(xs.OPERANDS) if operands is None: raise IsaAdditionError( @@ -686,25 +1037,43 @@ def _validate_identifier_addition( ) +def _expanded_opcodes(encoding: _EncodingDefinition, base_opcode: int) -> set[int]: + """Return all complete opcodes represented by a decoded identifier opcode.""" + base_count = 1 << encoding.opcode_bit_count + return { + base_opcode + modifier * base_count + for modifier in range(1 << encoding.opcode_modifier_bit_count) + } + + +def _identifier_owners( + addition: _IdentifierAddition, + opcode_owners: Mapping[tuple[str, int], str], +) -> set[str]: + return { + owner + for opcode in _expanded_opcodes(addition.encoding, addition.opcode) + if (owner := opcode_owners.get((addition.encoding.name, opcode))) is not None + } + + def _reachable_opcodes( encodings: Mapping[str, _EncodingDefinition], identifiers: Sequence[_IdentifierAddition], ) -> dict[str, set[int]]: - reachable = { + base_opcodes = { name: set(encoding.decoded_opcodes) for name, encoding in encodings.items() } for addition in identifiers: - reachable[addition.encoding.name].add(addition.opcode) - for name, encoding in encodings.items(): - if encoding.opcode_modifier_bit_count == 0: - continue - base_count = 1 << encoding.opcode_bit_count - base_opcodes = tuple(reachable[name]) - for modifier in range(1, 1 << encoding.opcode_modifier_bit_count): - reachable[name].update( - opcode + modifier * base_count for opcode in base_opcodes - ) - return reachable + base_opcodes[addition.encoding.name].add(addition.opcode) + return { + name: { + expanded + for opcode in base_opcodes[name] + for expanded in _expanded_opcodes(encoding, opcode) + } + for name, encoding in encodings.items() + } def apply_isa_additions( @@ -729,6 +1098,8 @@ def apply_isa_additions( opcode_owners, operand_types, instruction_names, + instruction_symbol_owners, + opcode_symbol_owners, ) = _base_definitions(base_root, profile) opcode_limits = { name: encoding.opcode_limit for name, encoding in encodings.items() @@ -760,11 +1131,15 @@ def apply_isa_additions( instruction, path=path, addition_id=addition_id, + encodings=encodings, encoding_names=encoding_names, opcode_owners=opcode_owners, opcode_limits=opcode_limits, operand_types=operand_types, instruction_names=instruction_names, + instruction_symbol_owners=instruction_symbol_owners, + opcode_symbol_owners=opcode_symbol_owners, + profile=profile, ) instruction_names.add(addition.name) for enc_name, opcode, _ in addition.forms: @@ -790,10 +1165,8 @@ def apply_isa_additions( occupied_texts=occupied_texts, occupied_opcodes=occupied_opcodes, ) - owner = addition_opcode_owners.get( - (addition.encoding.name, addition.opcode) - ) - if owner is None: + owners = _identifier_owners(addition, addition_opcode_owners) + if not owners: raise IsaAdditionError( f'{path}: additions document {addition_id!r} identifier for encoding ' f'{addition.encoding.name!r} opcode {addition.opcode} is ' @@ -801,18 +1174,26 @@ def apply_isa_additions( ) if addition.encoding.is_implied_literal: parent_name = addition.encoding.parent_name - parent_owner = addition_opcode_owners.get( - (parent_name, addition.opcode) - ) - if parent_owner != owner: + assert parent_name is not None + parent_encoding = encodings[parent_name] + parent_owners = { + owner + for opcode in _expanded_opcodes(parent_encoding, addition.opcode) + if (owner := addition_opcode_owners.get((parent_name, opcode))) + is not None + } + if parent_owners != owners: + owner_text = ', '.join(sorted(owners)) parent_owner_text = ( - repr(parent_owner) if parent_owner is not None else 'no owner' + ', '.join(sorted(parent_owners)) + if parent_owners + else 'no owner' ) raise IsaAdditionError( f'{path}: additions document {addition_id!r} ' 'implied-literal identifier ' f'for {addition.encoding.name!r} opcode {addition.opcode} ' - f'is owned by {owner!r}, but parent encoding ' + f'is owned by {owner_text!r}, but parent encoding ' f'{parent_name!r} has {parent_owner_text}' ) pending_identifiers.append(addition) diff --git a/emulation/rocjitsu/lib/python/amdisa/isa_profile.py b/emulation/rocjitsu/lib/python/amdisa/isa_profile.py index 64d53abbc185..f6154f20cbd5 100644 --- a/emulation/rocjitsu/lib/python/amdisa/isa_profile.py +++ b/emulation/rocjitsu/lib/python/amdisa/isa_profile.py @@ -440,6 +440,11 @@ def inst_size_overrides(self) -> dict[str, int]: """Per-instruction size overrides in bytes.""" return {} + @property + def compatibility_instruction_slots(self) -> dict[tuple[str, int], str]: + """Opcode slots owned by instructions synthesized after XML parsing.""" + return {} + @property def vop3px2_prefix_opcode(self) -> int | None: """VOP3P opcode slot for a VOP3PX2 prefix decoder, if any.""" @@ -2146,6 +2151,10 @@ def waitcnt_decode(self) -> str: """ return Rdna3Profile.waitcnt_decode.fget(self) + @property + def compatibility_instruction_slots(self) -> dict[tuple[str, int], str]: + return {('ENC_SOPP', 9): 'S_WAITCNT'} + @property def supported_versions(self) -> list[str]: return ['1.1.0', '1.1.1'] @@ -2463,6 +2472,10 @@ def field_renames(self, enc_name: str) -> dict[str, str]: renames['literal'] = 'simm32' return renames + @property + def compatibility_instruction_slots(self) -> dict[tuple[str, int], str]: + return {('ENC_VOP1', 103): 'V_PERMLANE64_B32'} + def normalize_operand_field_name(self, enc_name: str, field_name: str) -> str: # Keep the concrete gfx1250 operand identity distinct from earlier # CDNA/RDNA specs; its local generator path maps it to the renamed diff --git a/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py index b0936fa5ae46..d8b7c24c7b92 100644 --- a/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py +++ b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py @@ -16,7 +16,7 @@ IsaAdditionError, apply_isa_additions, ) -from amdisa.isa_profile import Cdna5Profile +from amdisa.isa_profile import Cdna1Profile, Cdna5Profile, Rdna4Profile from amdisa.parser import Parser _BASE_XML = '''\ @@ -54,17 +54,34 @@ + + false + false + false + false + false + BASE_INST + Base test instruction. ENC_TEST default - 1 + 1 - OPR_TEST + + FMT_NUM_B32 + OPR_TEST + 32 + + + TEST + TEST + @@ -116,17 +133,34 @@ def _instruction( ) -> str: return f'''\ + + false + false + false + false + false + {name} + Added test instruction. {encoding} default - {opcode} + {opcode} - {operand_type} + + FMT_NUM_B32 + {operand_type} + 32 + + + TEST + TEST + ''' @@ -352,6 +386,65 @@ def test_duplicate_instruction_across_additions_is_rejected(tmp_path): apply_isa_additions(_base_root(), [str(first), str(second)], _PROFILE) +def test_instruction_name_collision_after_codegen_normalization_is_rejected(tmp_path): + addition = _write_additions(tmp_path, _instruction('base_inst', opcode=2)) + + with pytest.raises( + IsaAdditionError, + match=r"base_inst.*/ENC_TEST.*symbol 'BaseInstTest'.*BASE_INST", + ): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + +def test_opcode_constant_name_collision_after_normalization_is_rejected(tmp_path): + root = _base_root() + root.find('./ISA/Instructions/Instruction/InstructionName').text = 'S_GETPC_B64' + addition = _write_additions(tmp_path, _instruction('S_GET_PC_B64', opcode=2)) + + with pytest.raises( + IsaAdditionError, + match=r"S_GET_PC_B64.*/ENC_TEST.*opcode constant symbol " + r"'kSGetPcB64Test'.*S_GETPC_B64", + ): + apply_isa_additions(root, [str(addition)], _PROFILE) + + +@pytest.mark.parametrize( + ('instruction', 'message'), + [ + ( + _instruction().replace('', ''), + 'unknown attributes: Unexpected', + ), + ( + _instruction().replace( + '', '' + ), + r'unknown elements: ', + ), + ( + _instruction().replace(' Input="true"', ''), + 'missing required attributes: Input', + ), + ( + _instruction().replace('32', ''), + r'expected exactly 1 ', + ), + ], +) +def test_incomplete_or_unknown_instruction_structure_is_rejected_before_merge( + tmp_path, instruction, message +): + root = _base_root() + before = elem_tree.tostring(root) + addition = _write_additions(tmp_path, instruction) + + with pytest.raises(IsaAdditionError, match=message): + apply_isa_additions(root, [str(addition)], _PROFILE) + + assert elem_tree.tostring(root) == before + + def test_same_instruction_may_repeat_slot_for_encoding_conditions(tmp_path): instructions = _instruction().replace( ' ', @@ -359,9 +452,14 @@ def test_same_instruction_may_repeat_slot_for_encoding_conditions(tmp_path): ENC_TEST alternate - 2 + 2 - OPR_TEST + + FMT_NUM_B32 + OPR_TEST + 32 + ''', @@ -629,6 +727,168 @@ def test_cli_additions_argument_requires_name_and_path(entry): _group_isa_additions_args([entry]) +def _write_encoding_clone_additions( + tmp_path: Path, + base_xml: Path, + *, + encoding_name: str, + instruction_name: str, + opcode: int, +) -> Path: + base_root = elem_tree.parse(base_xml).getroot() + original = next( + instruction + for instruction in base_root.findall('./ISA/Instructions/Instruction') + if any( + encoding.findtext('EncodingName') == encoding_name + for encoding in instruction.findall( + './InstructionEncodings/InstructionEncoding' + ) + ) + ) + addition = deepcopy(original) + addition.find('InstructionName').text = instruction_name + instruction_encodings = addition.find('InstructionEncodings') + for encoding in list(instruction_encodings): + if encoding.findtext('EncodingName') != encoding_name: + instruction_encodings.remove(encoding) + else: + encoding.find('Opcode').text = str(opcode) + + addition_root = elem_tree.Element( + 'IsaAdditions', + { + 'Id': 'compat-slot-test', + 'BaseArchitecture': base_root.findtext( + './ISA/Architecture/ArchitectureName' + ), + 'BaseSchemaVersion': base_root.findtext('./Document/SchemaVersion'), + }, + ) + instructions = elem_tree.SubElement(addition_root, 'InstructionAdditions') + instructions.append(addition) + path = tmp_path / 'compat-slot-test.xml' + elem_tree.ElementTree(addition_root).write(path, encoding='unicode') + return path + + +@pytest.mark.parametrize( + ('base_filename', 'profile', 'encoding_name', 'opcode', 'owner'), + [ + ( + 'amdgpu_isa_cdna5.xml', + Cdna5Profile(), + 'ENC_VOP1', + 103, + 'V_PERMLANE64_B32', + ), + ('amdgpu_isa_rdna4.xml', Rdna4Profile(), 'ENC_SOPP', 9, 'S_WAITCNT'), + ], +) +def test_compatibility_instruction_slots_are_reserved_before_merge( + tmp_path, base_filename, profile, encoding_name, opcode, owner +): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / base_filename + ) + addition = _write_encoding_clone_additions( + tmp_path, + base_xml, + encoding_name=encoding_name, + instruction_name='COLLIDING_ADDITION', + opcode=opcode, + ) + root = elem_tree.parse(base_xml).getroot() + before = elem_tree.tostring(root) + + with pytest.raises( + IsaAdditionError, + match=rf'opcode {opcode}.*already owned by instruction {owner!r}', + ): + apply_isa_additions(root, [str(addition)], profile) + + assert elem_tree.tostring(root) == before + + +def _cdna1_mimg_identifier(opcode: int) -> str: + value = list('00000000000000000000000000000000' '11110000000000000000000000000000') + value[39:46] = f'{opcode:07b}' + return ''.join(value) + + +def _write_cdna1_mimg_opm_additions(tmp_path: Path, base_xml: Path) -> Path: + base_root = elem_tree.parse(base_xml).getroot() + original = next( + instruction + for instruction in base_root.findall('./ISA/Instructions/Instruction') + if any( + encoding.findtext('EncodingName') == 'ENC_MIMG' + for encoding in instruction.findall( + './InstructionEncodings/InstructionEncoding' + ) + ) + ) + addition = deepcopy(original) + addition.find('InstructionName').text = 'IMAGE_ADDITION_TEST' + instruction_encodings = addition.find('InstructionEncodings') + for encoding in list(instruction_encodings): + if encoding.findtext('EncodingName') != 'ENC_MIMG': + instruction_encodings.remove(encoding) + else: + encoding.find('Opcode').text = '134' + + addition_root = elem_tree.Element( + 'IsaAdditions', + { + 'Id': 'cdna1-mimg-opm-test', + 'BaseArchitecture': 'AMD CDNA 1', + 'BaseSchemaVersion': '1.1.1', + }, + ) + identifier_additions = elem_tree.SubElement( + addition_root, 'EncodingIdentifierAdditions' + ) + identifier_addition = elem_tree.SubElement( + identifier_additions, 'EncodingIdentifierAddition' + ) + elem_tree.SubElement(identifier_addition, 'EncodingName').text = 'ENC_MIMG' + elem_tree.SubElement(identifier_addition, 'Opcode').text = '6' + identifier = elem_tree.SubElement( + identifier_addition, 'EncodingIdentifier', {'Radix': '2'} + ) + identifier.text = _cdna1_mimg_identifier(6) + instructions = elem_tree.SubElement(addition_root, 'InstructionAdditions') + instructions.append(addition) + path = tmp_path / 'cdna1-mimg-opm-test.xml' + elem_tree.ElementTree(addition_root).write(path, encoding='unicode') + return path + + +def test_identifier_ownership_uses_expanded_opm_opcode(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna1.xml' + ) + addition = _write_cdna1_mimg_opm_additions(tmp_path, base_xml) + + spec = Parser(str(base_xml), Cdna1Profile(), [str(addition)]).parse() + + added = next( + instruction + for instruction in spec.encoding_map['ENC_MIMG'].insts + if instruction.name == 'IMAGE_ADDITION_TEST' + ) + assert added.opcode == 134 + assert spec.encoding_map['ENC_MIMG'].primary_dt_ptrs[134] != -1 + + def _cdna5_sop1_identifier(opcode: int, *, implied_literal: bool = False) -> str: if implied_literal: value = list( @@ -668,6 +928,7 @@ def _write_cdna5_clone_additions( base_xml: Path, *, opcode: int = 7, + instruction_name: str = 'S_ADDITION_TEST_B32', add_identifiers: bool = False, include_implied_literal: bool = False, ) -> Path: @@ -678,7 +939,7 @@ def _write_cdna5_clone_additions( if instruction.findtext('InstructionName') == 'S_MOV_B32' ) addition = deepcopy(original) - addition.find('InstructionName').text = 'S_ADDITION_TEST_B32' + addition.find('InstructionName').text = instruction_name retained_encodings = {'ENC_SOP1'} if include_implied_literal: retained_encodings.add('SOP1_INST_LITERAL') @@ -729,6 +990,29 @@ def _write_cdna5_clone_additions( return path +def test_cdna5_normalized_instruction_name_collision_is_rejected(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions( + tmp_path, + base_xml, + instruction_name='s_mov_b32', + ) + + with pytest.raises( + IsaAdditionError, + match=r"s_mov_b32.*/ENC_SOP1.*symbol 'SMovB32Sop1'.*S_MOV_B32", + ): + apply_isa_additions( + elem_tree.parse(base_xml).getroot(), [str(addition)], Cdna5Profile() + ) + + def test_parser_retains_additions_provenance_on_added_instructions(tmp_path): base_xml = ( Path(__file__).resolve().parents[6] From 659f5f0c5f126e0892bb56b8c4119ca1ee7d6eec Mon Sep 17 00:00:00 2001 From: hanhanW Date: Wed, 26 Aug 2026 16:38:10 -0700 Subject: [PATCH 3/5] [rocjitsu] Reject unknown ISA addition provenance The additions reachability check assumed every internal provenance attribute was created by the current apply operation. A base XML carrying an unknown reserved attribute instead raised a bare KeyError and hid the malformed input. Check provenance IDs explicitly and report the XML path, instruction, and unknown value in an IsaAdditionError. Cover the failure with a CDNA5 parser regression that injects the reserved attribute into the base XML. --- .../rocjitsu/lib/python/amdisa/parser.py | 8 ++++++- .../python/amdisa/tests/test_isa_additions.py | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/emulation/rocjitsu/lib/python/amdisa/parser.py b/emulation/rocjitsu/lib/python/amdisa/parser.py index f0324bca6d41..cee277e79d73 100644 --- a/emulation/rocjitsu/lib/python/amdisa/parser.py +++ b/emulation/rocjitsu/lib/python/amdisa/parser.py @@ -370,8 +370,14 @@ def _validate_addition_decode_reachability(self) -> None: addition_id = inst_node.attrib.get(ADDITION_SOURCE_ATTR) if addition_id is None: continue - provenance = self._addition_by_id[addition_id] inst_name = xs.get_node_text(xs.get_node(inst_node, xs.INST_NAME)) + provenance = self._addition_by_id.get(addition_id) + if provenance is None: + raise IsaAdditionError( + f'{self.isa_xml}: instruction {inst_name!r} has unknown ' + f'{ADDITION_SOURCE_ATTR} value {addition_id!r}; the attribute ' + 'was not produced by a configured ISA additions document' + ) for inst_enc_node in xs.get_node(inst_node, xs.INST_ENCODINGS): enc_name = xs.get_node_text( xs.get_node(inst_enc_node, xs.ENCODING_NAME) diff --git a/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py index d8b7c24c7b92..7daf36d5dcf1 100644 --- a/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py +++ b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py @@ -1013,6 +1013,28 @@ def test_cdna5_normalized_instruction_name_collision_is_rejected(tmp_path): ) +def test_parser_rejects_unknown_base_addition_provenance(tmp_path): + source_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + tree = elem_tree.parse(source_xml) + instruction = tree.find('./ISA/Instructions/Instruction') + instruction_name = instruction.findtext('InstructionName') + instruction.set(ADDITION_SOURCE_ATTR, 'unknown-addition') + base_xml = tmp_path / 'base-with-unknown-provenance.xml' + tree.write(base_xml, encoding='unicode') + + with pytest.raises( + IsaAdditionError, + match=rf'{instruction_name}.*{ADDITION_SOURCE_ATTR}.*unknown-addition', + ): + Parser(str(base_xml), Cdna5Profile()).parse() + + def test_parser_retains_additions_provenance_on_added_instructions(tmp_path): base_xml = ( Path(__file__).resolve().parents[6] From a5835d6d9b57a984801668739c2685e756bf875a Mon Sep 17 00:00:00 2001 From: hanhanW Date: Thu, 27 Aug 2026 15:27:02 -0700 Subject: [PATCH 4/5] address comments Signed-off-by: hanhanW --- emulation/rocjitsu/docs/codegen.md | 19 ++- .../lib/python/amdisa/isa_additions.py | 157 ++++++++++++++++-- .../python/amdisa/tests/test_isa_additions.py | 119 ++++++++++++- 3 files changed, 269 insertions(+), 26 deletions(-) diff --git a/emulation/rocjitsu/docs/codegen.md b/emulation/rocjitsu/docs/codegen.md index 28bf8eaa2718..da7862b39be0 100644 --- a/emulation/rocjitsu/docs/codegen.md +++ b/emulation/rocjitsu/docs/codegen.md @@ -83,19 +83,24 @@ than a general XML patch: encoding's `` list are accepted. An additions document cannot replace or delete base definitions, identifier masks, microcode formats, encoding conditions, operand types, or data formats. -- Added instructions must reference encodings and operand types already - defined by the base XML. +- Added instructions must reference encodings, encoding conditions, operand + types, and operand fields already defined by the applicable base encoding + layout. Established implied-literal operands are validated against their + parent layout and literal-extension contract. - An identifier addition names its encoding and expected opcode. Its radix and width must match the base encoding; its fixed bits must match an existing identifier layout; and its decoded opcode must equal the declared opcode. Duplicate and colliding decode slots are rejected. Each identifier must be owned by an added instruction. An implied-literal identifier must have the same instruction owner and opcode in its parent encoding. -- Additions document IDs and instruction names must be unique. Encoding/opcode - ownership collisions between different instructions, repeated instruction - forms, out-of-range opcodes, missing references, unknown root data, and base - architecture/schema mismatches are errors. Repetition of one instruction's - slot under distinct MR-ISA encoding conditions is allowed. +- Additions document IDs and instruction names must be valid and unique. + Encoding/opcode ownership collisions between different instructions, + repeated instruction forms, multiple active forms that generate the same C++ + instruction symbol, out-of-range opcodes, missing references, unknown root + data, and base architecture/schema mismatches are errors. Repetition of one + instruction's slot under distinct base-established MR-ISA encoding conditions + is allowed only when profile filtering leaves at most one generated form + active. - Every input document and contained addition validates before the parser-owned tree is changed. Validated identifiers are merged before normal encoding/decode-table parsing, then instructions are appended in deterministic diff --git a/emulation/rocjitsu/lib/python/amdisa/isa_additions.py b/emulation/rocjitsu/lib/python/amdisa/isa_additions.py index 8dacd70d7fa4..3fff1dbe21c3 100644 --- a/emulation/rocjitsu/lib/python/amdisa/isa_additions.py +++ b/emulation/rocjitsu/lib/python/amdisa/isa_additions.py @@ -39,6 +39,10 @@ ADDITIONS_BASE_SCHEMA_ATTR, } _ID_PATTERN = re.compile(r'[A-Za-z0-9][A-Za-z0-9._-]*\Z') +_INSTRUCTION_NAME_PATTERN = re.compile(r'[A-Za-z][A-Za-z0-9_]*\Z') +# These operands name extension words that the generator patches explicitly; +# they are intentionally absent from the implied-literal form's parent bitmap. +_IMPLIED_LITERAL_OPERAND_FIELDS = frozenset({'literal', 'literal64', 'simm32'}) class IsaAdditionError(ValueError): @@ -47,7 +51,7 @@ class IsaAdditionError(ValueError): @dataclass(frozen=True) class _EncodingDefinition: - """Base-owned encoding information needed to validate identifier additions.""" + """Base-owned encoding information needed to validate ISA additions.""" name: str identifiers_node: elem_tree.Element @@ -62,6 +66,8 @@ class _EncodingDefinition: base_encoding_values: frozenset[int] identifier_texts: frozenset[str] decoded_opcodes: frozenset[int] + condition_ids: dict[str, frozenset[str]] + operand_field_names: frozenset[str] parent_name: str | None is_implied_literal: bool @@ -412,17 +418,19 @@ def _binary_text(node: elem_tree.Element, bit_width: int, context: str) -> str: return value -def _field_widths( +def _field_info( encoding: elem_tree.Element, profile: IsaProfile, context: str -) -> tuple[int, int, int]: +) -> tuple[int, int, int, frozenset[str]]: enc_name = _required_text(encoding, xs.ENCODING_NAME, context) renames = profile.field_renames(enc_name.upper()) enc_field_bits: int | None = None op_field_bits = 0 opm_field_bits = 0 + field_names: set[str] = set() for field in encoding.findall(f'./{xs.UCODE_FMT}/{xs.BITMAP}/{xs.FIELD}'): field_name = _required_text(field, xs.FIELD_NAME, context).lower() field_name = renames.get(field_name, field_name) + field_names.add(field_name) ranges = sorted( field.findall(f'{xs.BIT_LAYOUT}/{xs.RANGE}'), key=lambda node: int(node.attrib.get('Order', 0)), @@ -442,7 +450,45 @@ def _field_widths( opm_field_bits = width if enc_field_bits is None: raise IsaAdditionError(f'{context}: microcode format has no encoding field') - return enc_field_bits, op_field_bits, opm_field_bits + return enc_field_bits, op_field_bits, opm_field_bits, frozenset(field_names) + + +def _condition_ids( + encoding: elem_tree.Element, context: str +) -> dict[str, frozenset[str]]: + conditions: dict[str, set[str]] = {} + for condition in encoding.findall(f'./{xs.ENCODING_CONDS}/{xs.ENCODING_COND}'): + name = _required_text(condition, xs.COND_NAME, context) + condition_id = condition.findtext('ConditionId') + ids = conditions.setdefault(name, set()) + if condition_id is not None and condition_id.strip(): + ids.add(condition_id.strip()) + return {name: frozenset(ids) for name, ids in conditions.items()} + + +def _applicable_condition_ids( + encoding: _EncodingDefinition, + encodings: Mapping[str, _EncodingDefinition], +) -> dict[str, frozenset[str]]: + """Return conditions accepted for an encoding by the parser's inheritance.""" + + applicable: dict[str, set[str]] = { + name: set(ids) for name, ids in encoding.condition_ids.items() + } + + def include(conditions: Mapping[str, frozenset[str]]) -> None: + for name, ids in conditions.items(): + applicable.setdefault(name, set()).update(ids) + + if encoding.parent_name is not None: + parent = encodings.get(encoding.parent_name) + if parent is not None: + include(parent.condition_ids) + else: + for child in encodings.values(): + if child.parent_name == encoding.name: + include(child.condition_ids) + return {name: frozenset(ids) for name, ids in applicable.items()} def _layout_signature(text: str, opcode_slice: tuple[int, int]) -> str: @@ -524,9 +570,10 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ raise IsaAdditionError( f'base encoding {name!r}: missing <{xs.ENCODING_IDENTIFERS}>' ) - enc_bits, op_bits, opm_bits = _field_widths( + enc_bits, op_bits, opm_bits, own_field_names = _field_info( encoding, profile, f'base encoding {name!r}' ) + condition_ids = _condition_ids(encoding, f'base encoding {name!r}') try: encoding_slice, opcode_slice, dont_care_bits = ( parse_encoding_identifier_mask( @@ -559,6 +606,7 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ parent_name: str | None = None is_implied_literal = False + operand_field_names = own_field_names if profile.is_alt_encoding(name): parent_name = profile.derive_parent_enc_name(name) parent = encoding_definitions.get(parent_name) @@ -566,15 +614,13 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ raise IsaAdditionError( f'base encoding {name!r}: parent {parent_name!r} must appear first' ) - condition_names = [ - (_required_text(condition, xs.COND_NAME, f'base encoding {name!r}'), '') - for condition in encoding.findall( - f'./{xs.ENCODING_CONDS}/{xs.ENCODING_COND}' - ) - ] + condition_names = [(condition_name, '') for condition_name in condition_ids] is_implied_literal = profile.is_implied_literal_encoding( name, condition_names, bit_width, parent.bit_width ) + operand_field_names = parent.operand_field_names + if not is_implied_literal: + operand_field_names |= own_field_names encoding_definitions[name] = _EncodingDefinition( name=name, @@ -590,6 +636,8 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ base_encoding_values=frozenset(encoding_values), identifier_texts=frozenset(texts), decoded_opcodes=frozenset(decoded_opcodes), + condition_ids=condition_ids, + operand_field_names=operand_field_names, parent_name=parent_name, is_implied_literal=is_implied_literal, ) @@ -628,6 +676,18 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ xs.ENCODING_COND, f'base instruction {name}/{enc_name}', ) + if encoding_definition is not None: + # Some older vendor XMLs reference compatibility conditions from + # instructions without declaring them under EncodingConditions. + # Treat those base-established spellings as authoritative while + # still rejecting novel addition-only typos. + ids = set(encoding_definition.condition_ids.get(condition, ())) + condition_node = encoding.find(xs.ENCODING_COND) + assert condition_node is not None + condition_id = condition_node.attrib.get('Id') + if condition_id is not None: + ids.add(condition_id) + encoding_definition.condition_ids[condition] = frozenset(ids) if encoding_definition is None or profile.skip_inst_encoding( enc_name, condition ): @@ -793,6 +853,11 @@ def _validate_instruction_addition( context = f'{path}: additions document {addition_id!r}' _validate_instruction_structure(instruction, f'{context}: <{xs.INST}>') name = _required_text(instruction, xs.INST_NAME, context) + if not _INSTRUCTION_NAME_PATTERN.fullmatch(name): + raise IsaAdditionError( + f'{context}: invalid instruction name {name!r}; use ASCII letters, ' + 'digits, and underscores, starting with a letter' + ) if name in instruction_names: raise IsaAdditionError(f'{context}: instruction {name!r} already exists') @@ -804,6 +869,7 @@ def _validate_instruction_addition( forms_in_order: list[tuple[str, int, str]] = [] forms: set[tuple[str, int, str]] = set() + active_instruction_forms: dict[str, tuple[str, int, str]] = {} for encoding in instruction_encodings: if encoding.tag != xs.INST_ENCODING: raise IsaAdditionError( @@ -852,6 +918,30 @@ def _validate_instruction_addition( xs.ENCODING_COND, f'{context}: instruction {name!r}/{enc_name}', ) + encoding_definition = encodings.get(enc_name) + if encoding_definition is not None: + applicable_conditions = _applicable_condition_ids( + encoding_definition, encodings + ) + if condition not in applicable_conditions: + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} references ' + f'unknown encoding condition {condition!r}' + ) + condition_node = encoding.find(xs.ENCODING_COND) + assert condition_node is not None + condition_id = condition_node.attrib.get('Id') + declared_ids = applicable_conditions[condition] + if ( + condition_id is not None + and declared_ids + and condition_id not in declared_ids + ): + expected = ', '.join(sorted(declared_ids)) + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} condition ' + f'{condition!r} has Id {condition_id!r}, expected {expected}' + ) form = (enc_name, opcode, condition) if form in forms: raise IsaAdditionError( @@ -861,19 +951,30 @@ def _validate_instruction_addition( forms.add(form) forms_in_order.append(form) - encoding_definition = encodings.get(enc_name) if encoding_definition is not None and not profile.skip_inst_encoding( enc_name, condition ): is_implied_literal = encoding_definition.is_implied_literal + instruction_symbol = format_instruction_name( + name, + enc_name, + is_implied_literal_enc=is_implied_literal, + ) + previous_form = active_instruction_forms.get(instruction_symbol) + if previous_form is not None: + previous_encoding, previous_opcode, previous_condition = previous_form + raise IsaAdditionError( + f'{context}: instruction {name!r} has multiple active forms ' + f'for generated instruction symbol {instruction_symbol!r}: ' + f'{previous_encoding!r} opcode {previous_opcode} condition ' + f'{previous_condition!r}, and {enc_name!r} opcode {opcode} ' + f'condition {condition!r}' + ) + active_instruction_forms[instruction_symbol] = form generated_symbols = ( ( 'instruction', - format_instruction_name( - name, - enc_name, - is_implied_literal_enc=is_implied_literal, - ), + instruction_symbol, instruction_symbol_owners, ), ( @@ -903,16 +1004,36 @@ def _validate_instruction_addition( f'<{xs.OPERANDS}>' ) for operand in operands: + operand_context = f'{context}: instruction {name!r}/{enc_name} operand' op_type = _required_text( operand, xs.OPERAND_TYPE, - f'{context}: instruction {name!r}/{enc_name} operand', + operand_context, ) if op_type not in operand_types: raise IsaAdditionError( f'{context}: instruction {name!r}/{enc_name} references ' f'unknown operand type {op_type!r}' ) + field_node = operand.find(xs.FIELD_NAME) + if field_node is None or encoding_definition is None: + continue + field_name = _required_text(operand, xs.FIELD_NAME, operand_context) + normalized_field_name = profile.normalize_operand_field_name( + enc_name, field_name.lower() + ) + is_implied_literal_field = ( + encoding_definition.is_implied_literal + and normalized_field_name in _IMPLIED_LITERAL_OPERAND_FIELDS + ) + if ( + normalized_field_name not in encoding_definition.operand_field_names + and not is_implied_literal_field + ): + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} references ' + f'unknown operand field {field_name!r}' + ) addition = deepcopy(instruction) addition.attrib[ADDITION_SOURCE_ATTR] = addition_id diff --git a/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py index 7daf36d5dcf1..87013901243f 100644 --- a/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py +++ b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py @@ -38,6 +38,16 @@ 10100000010000000000000000000000 10100000010000000000000000000001 + + + default + 0 + + + alternate + 1 + + @@ -396,6 +406,17 @@ def test_instruction_name_collision_after_codegen_normalization_is_rejected(tmp_ apply_isa_additions(_base_root(), [str(addition)], _PROFILE) +def test_invalid_instruction_name_is_rejected_before_merge(tmp_path): + root = _base_root() + before = elem_tree.tostring(root) + addition = _write_additions(tmp_path, _instruction('BAD-NAME', opcode=2)) + + with pytest.raises(IsaAdditionError, match=r"invalid instruction name 'BAD-NAME'"): + apply_isa_additions(root, [str(addition)], _PROFILE) + + assert elem_tree.tostring(root) == before + + def test_opcode_constant_name_collision_after_normalization_is_rejected(tmp_path): root = _base_root() root.find('./ISA/Instructions/Instruction/InstructionName').text = 'S_GETPC_B64' @@ -445,7 +466,7 @@ def test_incomplete_or_unknown_instruction_structure_is_rejected_before_merge( assert elem_tree.tostring(root) == before -def test_same_instruction_may_repeat_slot_for_encoding_conditions(tmp_path): +def test_same_instruction_may_repeat_slot_when_only_one_condition_is_active(tmp_path): instructions = _instruction().replace( ' ', '''\ @@ -473,6 +494,34 @@ def test_same_instruction_may_repeat_slot_for_encoding_conditions(tmp_path): assert len(added.findall('./InstructionEncodings/InstructionEncoding')) == 2 +def test_unknown_encoding_condition_is_rejected_before_merge(tmp_path): + root = _base_root() + before = elem_tree.tostring(root) + instruction = _instruction().replace( + 'default', + 'defualt', + ) + addition = _write_additions(tmp_path, instruction) + + with pytest.raises(IsaAdditionError, match=r"unknown encoding condition 'defualt'"): + apply_isa_additions(root, [str(addition)], _PROFILE) + + assert elem_tree.tostring(root) == before + + +def test_encoding_condition_id_must_match_declared_condition(tmp_path): + instruction = _instruction().replace( + 'default', + 'default', + ) + addition = _write_additions(tmp_path, instruction) + + with pytest.raises( + IsaAdditionError, match=r"condition 'default' has Id '99', expected 0" + ): + apply_isa_additions(_base_root(), [str(addition)], _PROFILE) + + def test_same_instruction_cannot_repeat_slot_for_same_encoding_condition(tmp_path): instruction = elem_tree.fromstring(_instruction()) encodings = instruction.find('InstructionEncodings') @@ -990,6 +1039,74 @@ def _write_cdna5_clone_additions( return path +def test_cdna5_distinct_active_conditions_cannot_generate_the_same_class(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions(tmp_path, base_xml) + tree = elem_tree.parse(addition) + encodings = tree.find('./InstructionAdditions/Instruction/InstructionEncodings') + primary = next( + encoding + for encoding in encodings + if encoding.findtext('EncodingName') == 'ENC_SOP1' + ) + duplicate = deepcopy(primary) + condition = duplicate.find('EncodingCondition') + condition.text = 'default' + condition.set('Id', '0') + encodings.append(duplicate) + tree.write(addition, encoding='unicode') + root = elem_tree.parse(base_xml).getroot() + before = elem_tree.tostring(root) + + with pytest.raises( + IsaAdditionError, + match=r"multiple active forms.*symbol 'SAdditionTestB32Sop1'", + ): + apply_isa_additions(root, [str(addition)], Cdna5Profile()) + + assert elem_tree.tostring(root) == before + + +def test_cdna5_unknown_operand_field_is_rejected_before_merge(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions(tmp_path, base_xml) + tree = elem_tree.parse(addition) + primary = next( + encoding + for encoding in tree.findall( + './InstructionAdditions/Instruction/InstructionEncodings/' + 'InstructionEncoding' + ) + if encoding.findtext('EncodingName') == 'ENC_SOP1' + ) + destination = next( + field + for field in primary.findall('./Operands/Operand/FieldName') + if field.text == 'SDST' + ) + destination.text = 'BOGUS' + tree.write(addition, encoding='unicode') + root = elem_tree.parse(base_xml).getroot() + before = elem_tree.tostring(root) + + with pytest.raises(IsaAdditionError, match=r"unknown operand field 'BOGUS'"): + apply_isa_additions(root, [str(addition)], Cdna5Profile()) + + assert elem_tree.tostring(root) == before + + def test_cdna5_normalized_instruction_name_collision_is_rejected(tmp_path): base_xml = ( Path(__file__).resolve().parents[6] From 5e48f1a0d1640d8afb4ce0ac18366d00dd389b41 Mon Sep 17 00:00:00 2001 From: hanhanW Date: Fri, 28 Aug 2026 16:39:34 -0700 Subject: [PATCH 5/5] [rocjitsu] Close ISA additions validation gaps An addition could validate but emit no instruction when every form was removed by profile filtering. The implied-literal field exception also accepted a non-literal operand under a literal spelling, which could silently replace an encoded selector with zero. Compatibility slot ownership and injection duplicated the same instruction triples and could drift. Require every added instruction to retain an active form. Derive implied-literal field, type, and role contracts from base XML, while allowing new value sizes and data formats needed by target deltas. Make compatibility injectors obtain their encoding, opcode, and instruction name from the profile-owned slot mapping. Add atomic regressions for skipped forms and invalid literal roles, positive coverage for new literal value shapes, and multi-slot compatibility coverage. Update the additions documentation to state these contracts. Signed-off-by: hanhanW --- emulation/rocjitsu/docs/codegen.md | 7 +- .../lib/python/amdisa/isa_additions.py | 105 +++++++++++--- .../rocjitsu/lib/python/amdisa/parser.py | 117 ++++++++++------ .../tests/test_generator_profile_gates.py | 27 ++++ .../python/amdisa/tests/test_isa_additions.py | 128 ++++++++++++++++++ 5 files changed, 318 insertions(+), 66 deletions(-) diff --git a/emulation/rocjitsu/docs/codegen.md b/emulation/rocjitsu/docs/codegen.md index da7862b39be0..f81918c181c9 100644 --- a/emulation/rocjitsu/docs/codegen.md +++ b/emulation/rocjitsu/docs/codegen.md @@ -85,8 +85,9 @@ than a general XML patch: formats, encoding conditions, operand types, or data formats. - Added instructions must reference encodings, encoding conditions, operand types, and operand fields already defined by the applicable base encoding - layout. Established implied-literal operands are validated against their - parent layout and literal-extension contract. + layout. Implied-literal operands outside that layout are accepted only when + their normalized field, type, direction, and parser-role flags match an + implied-literal contract established by the base XML. - An identifier addition names its encoding and expected opcode. Its radix and width must match the base encoding; its fixed bits must match an existing identifier layout; and its decoded opcode must equal the declared opcode. @@ -100,7 +101,7 @@ than a general XML patch: data, and base architecture/schema mismatches are errors. Repetition of one instruction's slot under distinct base-established MR-ISA encoding conditions is allowed only when profile filtering leaves at most one generated form - active. + active. Every added instruction must retain at least one active form. - Every input document and contained addition validates before the parser-owned tree is changed. Validated identifiers are merged before normal encoding/decode-table parsing, then instructions are appended in deterministic diff --git a/emulation/rocjitsu/lib/python/amdisa/isa_additions.py b/emulation/rocjitsu/lib/python/amdisa/isa_additions.py index 3fff1dbe21c3..e22276e04802 100644 --- a/emulation/rocjitsu/lib/python/amdisa/isa_additions.py +++ b/emulation/rocjitsu/lib/python/amdisa/isa_additions.py @@ -40,15 +40,24 @@ } _ID_PATTERN = re.compile(r'[A-Za-z0-9][A-Za-z0-9._-]*\Z') _INSTRUCTION_NAME_PATTERN = re.compile(r'[A-Za-z][A-Za-z0-9_]*\Z') -# These operands name extension words that the generator patches explicitly; -# they are intentionally absent from the implied-literal form's parent bitmap. -_IMPLIED_LITERAL_OPERAND_FIELDS = frozenset({'literal', 'literal64', 'simm32'}) class IsaAdditionError(ValueError): """An ISA additions document is malformed or conflicts with its base XML.""" +@dataclass(frozen=True) +class _ImpliedLiteralOperandContract: + """Identity and role of an implied-literal operand established by the base.""" + + field_name: str + operand_type: str + is_input: bool + is_output: bool + is_implicit: bool + is_binary_microcode_required: bool + + @dataclass(frozen=True) class _EncodingDefinition: """Base-owned encoding information needed to validate ISA additions.""" @@ -68,6 +77,7 @@ class _EncodingDefinition: decoded_opcodes: frozenset[int] condition_ids: dict[str, frozenset[str]] operand_field_names: frozenset[str] + implied_literal_operand_contracts: set[_ImpliedLiteralOperandContract] parent_name: str | None is_implied_literal: bool @@ -143,6 +153,29 @@ def _required_decimal(text: str, context: str) -> int: return value +def _implied_literal_operand_contract( + operand: elem_tree.Element, + enc_name: str, + profile: IsaProfile, + context: str, +) -> _ImpliedLiteralOperandContract: + raw_field_name = _required_text(operand, xs.FIELD_NAME, context).lower() + operand_type = _required_text(operand, xs.OPERAND_TYPE, context) + return _ImpliedLiteralOperandContract( + field_name=profile.normalize_operand_field_name(enc_name, raw_field_name), + operand_type=profile.normalize_operand_type( + enc_name, raw_field_name, operand_type + ), + is_input=operand.attrib[xs.OPERAND_ATTR_INPUT].lower() == 'true', + is_output=operand.attrib[xs.OPERAND_ATTR_OUTPUT].lower() == 'true', + is_implicit=operand.attrib[xs.OPERAND_ATTR_IS_IMPLICIT].lower() == 'true', + is_binary_microcode_required=( + operand.attrib[xs.OPERAND_ATTR_IS_BINARY_MICROCODE_REQUIRED].lower() + == 'true' + ), + ) + + def _validate_node_shape( node: elem_tree.Element, context: str, @@ -638,6 +671,7 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ decoded_opcodes=frozenset(decoded_opcodes), condition_ids=condition_ids, operand_field_names=operand_field_names, + implied_literal_operand_contracts=set(), parent_name=parent_name, is_implied_literal=is_implied_literal, ) @@ -688,6 +722,25 @@ def _base_definitions(root: elem_tree.Element, profile: IsaProfile) -> tuple[ if condition_id is not None: ids.add(condition_id) encoding_definition.condition_ids[condition] = frozenset(ids) + if encoding_definition.is_implied_literal: + operands = encoding.find(xs.OPERANDS) + if operands is not None: + for operand in operands: + if operand.find(xs.FIELD_NAME) is None: + continue + contract = _implied_literal_operand_contract( + operand, + enc_name, + profile, + f'base instruction {name}/{enc_name} operand', + ) + if ( + contract.field_name + not in encoding_definition.operand_field_names + ): + encoding_definition.implied_literal_operand_contracts.add( + contract + ) if encoding_definition is None or profile.skip_inst_encoding( enc_name, condition ): @@ -1019,21 +1072,27 @@ def _validate_instruction_addition( if field_node is None or encoding_definition is None: continue field_name = _required_text(operand, xs.FIELD_NAME, operand_context) - normalized_field_name = profile.normalize_operand_field_name( - enc_name, field_name.lower() + contract = _implied_literal_operand_contract( + operand, enc_name, profile, operand_context ) - is_implied_literal_field = ( - encoding_definition.is_implied_literal - and normalized_field_name in _IMPLIED_LITERAL_OPERAND_FIELDS - ) - if ( - normalized_field_name not in encoding_definition.operand_field_names - and not is_implied_literal_field - ): + if contract.field_name in encoding_definition.operand_field_names: + continue + if contract in encoding_definition.implied_literal_operand_contracts: + continue + established_implied_literal_fields = { + item.field_name + for item in encoding_definition.implied_literal_operand_contracts + } + if contract.field_name in established_implied_literal_fields: raise IsaAdditionError( f'{context}: instruction {name!r}/{enc_name} references ' - f'unknown operand field {field_name!r}' + f'implied-literal operand field {field_name!r} with a type or ' + 'role not established by the base XML' ) + raise IsaAdditionError( + f'{context}: instruction {name!r}/{enc_name} references ' + f'unknown operand field {field_name!r}' + ) addition = deepcopy(instruction) addition.attrib[ADDITION_SOURCE_ATTR] = addition_id @@ -1321,11 +1380,19 @@ def apply_isa_additions( reachable = _reachable_opcodes(encodings, pending_identifiers) for addition in pending_instructions: - for enc_name, opcode, condition in addition.forms: - if enc_name in profile.skip_encodings or profile.skip_inst_encoding( - enc_name, condition - ): - continue + active_forms = [ + form + for form in addition.forms + if form[0] not in profile.skip_encodings + and not profile.skip_inst_encoding(form[0], form[2]) + ] + if not active_forms: + raise IsaAdditionError( + f'{addition.path}: additions document {addition.addition_id!r} ' + f'instruction {addition.name!r} has no encoding form active in ' + 'the selected ISA profile' + ) + for enc_name, opcode, _condition in active_forms: if opcode not in reachable[enc_name]: raise IsaAdditionError( f'{addition.path}: additions document ' diff --git a/emulation/rocjitsu/lib/python/amdisa/parser.py b/emulation/rocjitsu/lib/python/amdisa/parser.py index cee277e79d73..21bf0dd4d279 100644 --- a/emulation/rocjitsu/lib/python/amdisa/parser.py +++ b/emulation/rocjitsu/lib/python/amdisa/parser.py @@ -462,6 +462,24 @@ def _inject_compat_insts(self) -> None: self._inject_s_waitcnt_compat() self._inject_cdna5_permlane64_compat() + def _compatibility_instruction_slot( + self, expected_instruction_name: str + ) -> tuple[str, int, str]: + """Return the profile-owned instruction triple for its injector.""" + slots = self.profile.compatibility_instruction_slots + matches = [ + (enc_name, opcode, instruction_name) + for (enc_name, opcode), instruction_name in slots.items() + if instruction_name == expected_instruction_name + ] + if len(matches) != 1: + raise ValueError( + f'{self.isa_spec.arch_name} profile must declare exactly one ' + f'compatibility instruction named {expected_instruction_name}, ' + f'found {matches}' + ) + return matches[0] + def _inject_s_waitcnt_compat(self) -> None: """Add the legacy monolithic S_WAITCNT accepted by LLVM on GFX12.""" if self.isa_spec.arch_name != 'rdna4': @@ -470,29 +488,35 @@ def _inject_s_waitcnt_compat(self) -> None: # The RDNA4/GFX12 XML only lists split S_WAIT_* instructions, but LLVM # still accepts and emits the monolithic SOPP opcode-9 S_WAITCNT # compatibility form for sources such as "s_waitcnt lgkmcnt(0)". - enc = self.isa_spec.encoding_map.get('ENC_SOPP') + enc_name, opcode, instruction_name = self._compatibility_instruction_slot( + 'S_WAITCNT' + ) + enc = self.isa_spec.encoding_map.get(enc_name) if enc is None: raise ValueError( - 'RDNA4 S_WAITCNT compatibility injection requires ENC_SOPP' + f'RDNA4 {instruction_name} compatibility injection requires ' + f'{enc_name}' ) if enc.primary_dt_ptrs is None: raise ValueError( - 'RDNA4 S_WAITCNT compatibility injection requires an ENC_SOPP ' - 'primary decode route table' + f'RDNA4 {instruction_name} compatibility injection requires an ' + f'{enc_name} primary decode route table' ) - opcode = 9 - if any(inst.name == 'S_WAITCNT' and inst.opcode == 9 for inst in enc.insts): + if any( + inst.name == instruction_name and inst.opcode == opcode + for inst in enc.insts + ): return occupied = next((inst for inst in enc.insts if inst.opcode == opcode), None) if occupied is not None: raise ValueError( - 'RDNA4 S_WAITCNT compatibility opcode 9 is already occupied by ' - f'{occupied.name}' + f'RDNA4 {instruction_name} compatibility opcode {opcode} is ' + f'already occupied by {occupied.name}' ) if len(enc.primary_dt_ptrs) <= opcode: raise ValueError( - 'RDNA4 ENC_SOPP primary decode route table does not contain ' - 'S_WAITCNT opcode 9' + f'RDNA4 {enc_name} primary decode route table does not contain ' + f'{instruction_name} opcode {opcode}' ) dt_ptr = enc.primary_dt_ptrs[opcode] @@ -509,31 +533,33 @@ def _inject_s_waitcnt_compat(self) -> None: } if not matching_dt_ptrs: raise ValueError( - 'RDNA4 S_WAITCNT opcode 9 has no ENC_SOPP primary decode route' + f'RDNA4 {instruction_name} opcode {opcode} has no {enc_name} ' + 'primary decode route' ) if len(matching_dt_ptrs) != 1: raise ValueError( - 'RDNA4 S_WAITCNT opcode 9 requires exactly one unique ' - 'ENC_SOPP primary decode route, found ' + f'RDNA4 {instruction_name} opcode {opcode} requires exactly ' + f'one unique {enc_name} primary decode route, found ' f'{sorted(matching_dt_ptrs)}' ) dt_ptr = matching_dt_ptrs.pop() if dt_ptr < 0 or dt_ptr >= len(self.isa_spec.primary_decode_table): raise ValueError( - 'RDNA4 S_WAITCNT opcode 9 resolves to invalid primary ' + f'RDNA4 {instruction_name} opcode {opcode} resolves to invalid primary ' f'decode-table index {dt_ptr}' ) dte = self.isa_spec.primary_decode_table[dt_ptr] - decode_func = 'decodeSWaitcntSopp' if dte.sub_decode_funcs is not None: if opcode >= len(dte.sub_decode_funcs): raise ValueError( - 'RDNA4 S_WAITCNT opcode 9 is outside the selected subdecode table' + f'RDNA4 {instruction_name} opcode {opcode} is outside the ' + 'selected subdecode table' ) if dte.sub_decode_funcs[opcode] not in (None, 'decodeInvalid'): raise ValueError( - 'RDNA4 S_WAITCNT opcode 9 subdecode slot is already occupied by ' + f'RDNA4 {instruction_name} opcode {opcode} subdecode slot is ' + 'already occupied by ' f'{dte.sub_decode_funcs[opcode]}' ) elif ( @@ -541,13 +567,13 @@ def _inject_s_waitcnt_compat(self) -> None: or getattr(dte, 'inst_name', None) is not None ): raise ValueError( - 'RDNA4 S_WAITCNT terminal decode entry is already occupied' + f'RDNA4 {instruction_name} terminal decode entry is already occupied' ) inst = Instruction( - 'S_WAITCNT', - 'ENC_SOPP', - 9, + instruction_name, + enc_name, + opcode, [ Operand( 'simm16', @@ -560,7 +586,7 @@ def _inject_s_waitcnt_compat(self) -> None: 1, ) ], - available_encodings=frozenset({'ENC_SOPP'}), + available_encodings=frozenset({enc_name}), ) insert_idx = next( ( @@ -594,32 +620,35 @@ def _inject_cdna5_permlane64_compat(self) -> None: if self.isa_spec.arch_name != 'cdna5': return - enc = self.isa_spec.encoding_map.get('ENC_VOP1') + enc_name, opcode, instruction_name = self._compatibility_instruction_slot( + 'V_PERMLANE64_B32' + ) + enc = self.isa_spec.encoding_map.get(enc_name) if enc is None: raise ValueError( - 'CDNA5 V_PERMLANE64_B32 compatibility injection requires ENC_VOP1' + f'CDNA5 {instruction_name} compatibility injection requires ' + f'{enc_name}' ) if enc.primary_dt_ptrs is None: raise ValueError( - 'CDNA5 V_PERMLANE64_B32 compatibility injection requires ' - 'an ENC_VOP1 primary decode route table' + f'CDNA5 {instruction_name} compatibility injection requires ' + f'an {enc_name} primary decode route table' ) - opcode = 103 if any( - inst.name == 'V_PERMLANE64_B32' and inst.opcode == opcode + inst.name == instruction_name and inst.opcode == opcode for inst in enc.insts ): return occupied = next((inst for inst in enc.insts if inst.opcode == opcode), None) if occupied is not None: raise ValueError( - 'CDNA5 V_PERMLANE64_B32 compatibility opcode 103 is already ' - f'occupied by {occupied.name}' + f'CDNA5 {instruction_name} compatibility opcode {opcode} is ' + f'already occupied by {occupied.name}' ) if len(enc.primary_dt_ptrs) <= opcode: raise ValueError( - 'CDNA5 ENC_VOP1 primary decode route table does not contain ' - 'V_PERMLANE64_B32 opcode 103' + f'CDNA5 {enc_name} primary decode route table does not contain ' + f'{instruction_name} opcode {opcode}' ) dt_ptr = enc.primary_dt_ptrs[opcode] @@ -633,40 +662,39 @@ def _inject_cdna5_permlane64_compat(self) -> None: } if len(adjacent_routes) != 1: raise ValueError( - 'CDNA5 V_PERMLANE64_B32 opcode 103 requires exactly one ' - 'adjacent ENC_VOP1 decode route' + f'CDNA5 {instruction_name} opcode {opcode} requires exactly ' + f'one adjacent {enc_name} decode route' ) dt_ptr = adjacent_routes.pop() if dt_ptr < 0 or dt_ptr >= len(self.isa_spec.primary_decode_table): raise ValueError( - 'CDNA5 V_PERMLANE64_B32 opcode 103 resolves to invalid primary ' + f'CDNA5 {instruction_name} opcode {opcode} resolves to invalid primary ' f'decode-table index {dt_ptr}' ) dte = self.isa_spec.primary_decode_table[dt_ptr] - decode_func = 'decodeVPermlane64B32Vop1' if dte.sub_decode_funcs is not None: if opcode >= len(dte.sub_decode_funcs): raise ValueError( - 'CDNA5 V_PERMLANE64_B32 opcode 103 is outside the selected ' - 'subdecode table' + f'CDNA5 {instruction_name} opcode {opcode} is outside the ' + 'selected subdecode table' ) if dte.sub_decode_funcs[opcode] not in (None, 'decodeInvalid'): raise ValueError( - 'CDNA5 V_PERMLANE64_B32 opcode 103 subdecode slot is already ' - f'occupied by {dte.sub_decode_funcs[opcode]}' + f'CDNA5 {instruction_name} opcode {opcode} subdecode slot is ' + f'already occupied by {dte.sub_decode_funcs[opcode]}' ) elif ( getattr(dte, 'decode_func', None) is not None or getattr(dte, 'inst_name', None) is not None ): raise ValueError( - 'CDNA5 V_PERMLANE64_B32 terminal decode entry is already occupied' + f'CDNA5 {instruction_name} terminal decode entry is already occupied' ) inst = Instruction( - 'V_PERMLANE64_B32', - 'ENC_VOP1', + instruction_name, + enc_name, opcode, [ Operand( @@ -692,7 +720,7 @@ def _inject_cdna5_permlane64_compat(self) -> None: 'FMT_NUM_B32', ), ], - available_encodings=frozenset({'ENC_VOP1'}), + available_encodings=frozenset({enc_name}), ) insert_idx = next( (idx for idx, existing in enumerate(enc.insts) if existing.opcode > opcode), @@ -702,6 +730,7 @@ def _inject_cdna5_permlane64_compat(self) -> None: if patch_route: enc.primary_dt_ptrs[opcode] = dt_ptr + decode_func = f'decode{inst.fmt_name}' if dte.sub_decode_funcs is not None: dte.sub_decode_funcs[opcode] = decode_func else: diff --git a/emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py b/emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py index a9e9e62a51d1..9e2d5b77177a 100644 --- a/emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py +++ b/emulation/rocjitsu/lib/python/amdisa/tests/test_generator_profile_gates.py @@ -1125,6 +1125,11 @@ def test_gfx1250_generated_inventory_omits_legacy_s_waitcnt( def _fake_sopp_parser(arch_name: str, *, sub_decode_funcs: list[str | None] | None): parser = object.__new__(Parser) + parser.profile = { + 'cdna5': Cdna5Profile(), + 'rdna3': Rdna3Profile(), + 'rdna4': Rdna4Profile(), + }[arch_name] sopp = SimpleNamespace( insts=[ Instruction('S_WAIT_ALU', 'ENC_SOPP', 8, []), @@ -1180,6 +1185,21 @@ def test_rdna4_parser_injects_s_waitcnt_compat_once_in_opcode_order(): assert dte.sub_decode_funcs[9] == 'decodeSWaitcntSopp' +def test_rdna4_parser_selects_its_compatibility_slot_by_instruction_name(): + parser, sopp, dte = _fake_sopp_parser('rdna4', sub_decode_funcs=[None] * 16) + parser.profile = SimpleNamespace( + compatibility_instruction_slots={ + ('ENC_OTHER', 4): 'OTHER_COMPAT', + ('ENC_SOPP', 9): 'S_WAITCNT', + } + ) + + parser._inject_s_waitcnt_compat() + + assert any(inst.name == 'S_WAITCNT' and inst.opcode == 9 for inst in sopp.insts) + assert dte.sub_decode_funcs[9] == 'decodeSWaitcntSopp' + + def test_gfx1250_parser_does_not_inject_legacy_s_waitcnt(): parser, sopp, dte = _fake_sopp_parser('cdna5', sub_decode_funcs=[None] * 16) @@ -1194,6 +1214,7 @@ def test_gfx1250_parser_does_not_inject_legacy_s_waitcnt(): def test_gfx1250_parser_injects_permlane64_compat_once(): parser = object.__new__(Parser) + parser.profile = Cdna5Profile() vop1 = SimpleNamespace( insts=[ Instruction('V_SWAP_B16', 'ENC_VOP1', 102, []), @@ -1230,6 +1251,7 @@ def test_gfx1250_parser_injects_permlane64_compat_once(): def test_gfx1250_parser_permlane64_requires_an_unambiguous_adjacent_route(): parser = object.__new__(Parser) + parser.profile = Cdna5Profile() vop1 = SimpleNamespace( insts=[Instruction('V_SWAP_B16', 'ENC_VOP1', 102, [])], primary_dt_ptrs=[-1] * 128, @@ -1256,6 +1278,7 @@ def test_gfx1250_parser_permlane64_requires_an_unambiguous_adjacent_route(): def test_gfx1250_parser_permlane64_rejects_opcode_collision_before_mutation(): parser = object.__new__(Parser) + parser.profile = Cdna5Profile() collision = Instruction('V_OTHER_B32', 'ENC_VOP1', 103, []) vop1 = SimpleNamespace( insts=[collision], @@ -1280,6 +1303,7 @@ def test_gfx1250_parser_permlane64_rejects_opcode_collision_before_mutation(): def test_gfx1250_parser_permlane64_rejects_invalid_decode_table_index_before_mutation(): parser = object.__new__(Parser) + parser.profile = Cdna5Profile() vop1 = SimpleNamespace( insts=[Instruction('V_SWAP_B16', 'ENC_VOP1', 102, [])], primary_dt_ptrs=[-1] * 128, @@ -1334,6 +1358,7 @@ def test_gfx1250_parser_permlane64_rejects_terminal_conflicts_before_mutation( decode_entry, message ): parser = object.__new__(Parser) + parser.profile = Cdna5Profile() neighbor = Instruction('V_SWAP_B16', 'ENC_VOP1', 102, []) vop1 = SimpleNamespace( insts=[neighbor], @@ -1371,6 +1396,7 @@ def test_gfx1250_parser_permlane64_rejects_missing_route_invariants( encoding_map, message ): parser = object.__new__(Parser) + parser.profile = Cdna5Profile() parser.isa_spec = SimpleNamespace( arch_name='cdna5', encoding_map=encoding_map, @@ -1418,6 +1444,7 @@ def test_rdna4_s_waitcnt_rejects_opcode_collision_before_mutation(): ) def test_rdna4_s_waitcnt_rejects_missing_route_invariants(encoding_map, message): parser = object.__new__(Parser) + parser.profile = Rdna4Profile() parser.isa_spec = SimpleNamespace( arch_name='rdna4', encoding_map=encoding_map, diff --git a/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py index 87013901243f..cfd9019dfa93 100644 --- a/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py +++ b/emulation/rocjitsu/lib/python/amdisa/tests/test_isa_additions.py @@ -494,6 +494,21 @@ def test_same_instruction_may_repeat_slot_when_only_one_condition_is_active(tmp_ assert len(added.findall('./InstructionEncodings/InstructionEncoding')) == 2 +def test_instruction_requires_an_active_encoding_condition(tmp_path): + root = _base_root() + before = elem_tree.tostring(root) + instruction = _instruction().replace( + 'default', + 'alternate', + ) + addition = _write_additions(tmp_path, instruction) + + with pytest.raises(IsaAdditionError, match=r'no encoding form active'): + apply_isa_additions(root, [str(addition)], _PROFILE) + + assert elem_tree.tostring(root) == before + + def test_unknown_encoding_condition_is_rejected_before_merge(tmp_path): root = _base_root() before = elem_tree.tostring(root) @@ -863,6 +878,30 @@ def test_compatibility_instruction_slots_are_reserved_before_merge( assert elem_tree.tostring(root) == before +def test_instruction_in_fully_skipped_encoding_is_rejected_before_merge(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_encoding_clone_additions( + tmp_path, + base_xml, + encoding_name='ENC_VOP3PX2', + instruction_name='SKIPPED_ADDITION', + opcode=52, + ) + root = elem_tree.parse(base_xml).getroot() + before = elem_tree.tostring(root) + + with pytest.raises(IsaAdditionError, match=r'no encoding form active'): + apply_isa_additions(root, [str(addition)], Cdna5Profile()) + + assert elem_tree.tostring(root) == before + + def _cdna1_mimg_identifier(opcode: int) -> str: value = list('00000000000000000000000000000000' '11110000000000000000000000000000') value[39:46] = f'{opcode:07b}' @@ -1107,6 +1146,95 @@ def test_cdna5_unknown_operand_field_is_rejected_before_merge(tmp_path): assert elem_tree.tostring(root) == before +@pytest.mark.parametrize( + ('field_name', 'message'), + [ + ('literal64', r"unknown operand field 'literal64'"), + ( + 'LITERAL', + r"implied-literal operand field 'LITERAL'.*type or role not established", + ), + ], +) +def test_cdna5_implied_literal_field_requires_an_established_contract( + tmp_path, field_name, message +): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions( + tmp_path, base_xml, include_implied_literal=True + ) + tree = elem_tree.parse(addition) + implied_literal = next( + encoding + for encoding in tree.findall( + './InstructionAdditions/Instruction/InstructionEncodings/' + 'InstructionEncoding' + ) + if encoding.findtext('EncodingName') == 'SOP1_INST_LITERAL' + ) + destination = next( + operand + for operand in implied_literal.findall('./Operands/Operand') + if operand.findtext('FieldName') == 'SDST' + ) + destination.find('FieldName').text = field_name + tree.write(addition, encoding='unicode') + root = elem_tree.parse(base_xml).getroot() + before = elem_tree.tostring(root) + + with pytest.raises(IsaAdditionError, match=message): + apply_isa_additions(root, [str(addition)], Cdna5Profile()) + + assert elem_tree.tostring(root) == before + + +def test_cdna5_implied_literal_contract_allows_a_new_value_shape(tmp_path): + base_xml = ( + Path(__file__).resolve().parents[6] + / 'shared' + / 'machine-readable-isa' + / 'isa' + / 'amdgpu_isa_cdna5.xml' + ) + addition = _write_cdna5_clone_additions( + tmp_path, + base_xml, + add_identifiers=True, + include_implied_literal=True, + ) + tree = elem_tree.parse(addition) + literal_operands = [ + operand + for encoding in tree.findall( + './InstructionAdditions/Instruction/InstructionEncodings/' + 'InstructionEncoding' + ) + if encoding.findtext('EncodingName') == 'SOP1_INST_LITERAL' + for operand in encoding.findall('./Operands/Operand') + if operand.findtext('FieldName') == 'LITERAL' + ] + assert literal_operands + for operand in literal_operands: + operand.find('OperandSize').text = '128' + operand.find('DataFormatName').text = 'FMT_NUM_PK2_F64' + tree.write(addition, encoding='unicode') + + spec = Parser(str(base_xml), Cdna5Profile(), [str(addition)]).parse() + + added = next( + instruction + for instruction in spec.encoding_map['ENC_SOP1'].insts + if instruction.name == 'S_ADDITION_TEST_B32' + ) + assert added.source_addition == spec.applied_additions[0] + + def test_cdna5_normalized_instruction_name_collision_is_rejected(tmp_path): base_xml = ( Path(__file__).resolve().parents[6]