In this document, we summarize the guidelines for reviewing changes to the specification. At the moment, these changes typically involve checking multiple things in multiple sources, so this document summarizes them all to simplify reviews:
- Check that the "Specification" column in status.md says "yes", add a row if adding a new op.
- Check if the section title matches the op's mnemonic in the ODS.
- Check if the "Semantics" section matches XLA's Operation Semantics.
- Check whether the "Inputs" and "Outputs" sections:
- List the same items as the ODS.
- List the same items as HloInstruction::CreateFromProto.
- Are ordered exactly like ODS.
- If there are any mismatches, check that there are corresponding tickets.
- Check whether the "Constraints" section:
- Matches XLA's shape_inference.cc.
- Matches XLA's hlo_verifier.cc.
- Matches the ODS.
- Matches StablehloOps.cpp.
- If there are any mismatches, check that there are corresponding tickets. Link all those tickets in the spec, in locations which are as specific as possible (e.g. if a ticket is about a constraint that hasn't been implemented, link the ticket right in that constraint).
- If the corresponding parts of the ODS and StablehloOps.cpp match the spec, check that the "Verification" and "Type Inference" columns in status.md say "yes".
- Check whether the "Examples" section:
- Only has one example. (In the future, we'll link to more examples from the StableHLO interpreter test suite).
- Uses valid MLIR syntax by running
stablehlo-opt
on code examples. - Uses generic MLIR syntax which can be obtained by running
stablehlo-opt -mlir-print-op-generic
(we stick to generic syntax in the spec to avoid having to change the spec on prettyprinter changes).
- Check that the
description
in the op's ODS:- Includes the first sentence of the spec.
- Then links to the corresponding section of the spec.
- Then uses the same example as the spec but via pretty syntax which can
be obtaining by running
stablehlo-opt
.
- Check that the files related to implementing verification and type
inference constraints follow the guidelines as mentioned below:
- Follow guideline #1 for StablehloOps.td.
- Follow guideline #2 for TypeInference.cpp and StablehloOps.cpp.
- Follow guideline #5 for ops_stablehlo.mlir.
- Follow guideline #6 for infer_stablehlo.mlir.
- Evaluate the op for side effects and
speculatability.
- If the op has no side effects and is always speculatable, give it the
Pure
trait. This is rare, as most ops allow dynamic shapes, which may lead to shape mismatches at runtime, which is undefined behavior. Some ops can have undefined behavior in other situations as well. The vast majority of ops do not have side effects (they should have theNoMemoryEffect
trait). - Most ops fall into one of the
HLO_SpeculatableIf*
traits. If the op does not fit into any of those, give it theConditionallySpeculatable
trait and implement the interface methods. Add tests tostablehlo/tests/ops_speculatability.mlir
to cover the speculatability logic.
- If the op has no side effects and is always speculatable, give it the