From bfea42289b25e9a9a3cd34a0276dabf66be3212a Mon Sep 17 00:00:00 2001 From: Kevin Gleason Date: Mon, 29 Jan 2024 10:18:17 -0600 Subject: [PATCH] Add instructions for opset change contributions to CONTRIBUTING.md (#1921) Although the RFC process under OpenXLA is not fully standardized yet, there have been a few changes to StableHLO lately which had success using a similar process. This PR adds some documentation on the steps they followed to `contributing.md`. --- CONTRIBUTING.md | 59 ++++++++++++++++++++++++++++++++++++++++++ docs/spec_checklist.md | 3 ++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c980350f8fc..63da8a3108c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,3 +21,62 @@ All submissions, including submissions by project members, require review. We use GitHub pull requests for this purpose. Consult [GitHub Help](https://help.github.com/articles/about-pull-requests/) for more information on using pull requests. + +## StableHLO Opset Changes + +All changes to the StableHLO opset including new ops, types, or attributes must +be reviewed via an RFC. We aim for StableHLO opset changes to take ~2 weeks +if feedback is actively addressed. This allows adequate time for the community +to review all proposals. + +### 1. Write an RFC + +An RFC should outline the proposed spec changes, as well as the rationale, and +alternatives considered if relevant. This can be shared as a markdown file in +the [`rfcs/`](https://github.com/openxla/stablehlo/tree/main/rfcs) directory and +shared as a PR. + +For example, see the [`collective_broadcast` RFC](https://github.com/openxla/stablehlo/pull/1809). + +### 2. Notify OpenXLA Discuss + +To signal boost your RFC, post on [OpenXLA Discuss](https://groups.google.com/a/openxla.org/g/openxla-discuss). +This will ensure the proper reviewers see the RFC. While there is no formal +process for posts, we tend to recommend keeping RFC discussion on the PR to keep +feedback centralized in the repository. + +For example, see the [`collective_broadcast` post](https://groups.google.com/a/openxla.org/g/openxla-discuss/c/Q7JFyoiVFPU/m/dUH_LmJlCgAJ). + +### 3. Work with project maintainer for final approval + +As denoted in [`governance.md`](https://github.com/openxla/stablehlo/blob/main/docs/governance.md), +while we work towards instating StableHLO module maintainers, the interim review +process requires approval from Google project maintainers. A member of the +StableHLO team will help drive final approval. + +### 4. Send PRs for the opset change + +Once an RFC is approved, PRs which implement the approved proposal may be sent, +reviewed, and merged. + +A few things to consider when adding new features: + +- Spec, Type Inference, Verifiers: Steps for adding to [`spec.md`](https://github.com/openxla/stablehlo/blob/main/docs/spec.md) +as well as related op implementation can be found in +[`spec_checklist.md`](https://github.com/openxla/stablehlo/blob/main/docs/spec_checklist.md). +- Verifiers: Steps for modifying or implementing op verifiers can also be found +in the spec checklist. +- Type Inference: Type inference design principles and testing details can be +found in [`type_inference.md`](https://github.com/openxla/stablehlo/blob/main/docs/type_inference.md). +- Compatibility: Tips on managing forward/backward compatibility are in +[`vhlo.md`](https://github.com/openxla/stablehlo/blob/main/docs/vhlo.md#contributing-incompatible-changes). +- Reference: Steps for adding interpreter support can be found in +[`reference_checklist.md`](https://github.com/openxla/stablehlo/blob/main/docs/reference_checklist.md). +- Tests: For each of the above modifications, consider positive and negative +test cases. + +Some examples to help guide changes: + +- Adding a new op: [`collective_broadcast`](https://github.com/openxla/stablehlo/pull/1856). +- Adding new types: [`f8E4M3FNUZ` and `f8E5M2FNUZ`](https://github.com/openxla/stablehlo/pull/1379). +- Expanding type support: [Quantized `ReduceOp`](https://github.com/openxla/stablehlo/pull/1796). diff --git a/docs/spec_checklist.md b/docs/spec_checklist.md index aa3ce827714..dc4b630c663 100644 --- a/docs/spec_checklist.md +++ b/docs/spec_checklist.md @@ -5,7 +5,8 @@ specification. At the moment, these changes typically involve checking multiple things in multiple sources, so this document summarizes them all to simplify reviews: - 1. Check that the "Specification" column in status.md says "yes". + 1. Check that the "Specification" column in status.md says "yes", add a row if + adding a new op. 1. Check if the section title matches the op's mnemonic in [the ODS](https://github.com/openxla/stablehlo/blob/main/stablehlo/dialect/StablehloOps.td). 1. Check if the "Semantics" section matches XLA's