Skip to content

Commit

Permalink
Add instructions for opset change contributions to CONTRIBUTING.md (#…
Browse files Browse the repository at this point in the history
…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`.
  • Loading branch information
GleasonK authored Jan 29, 2024
1 parent 00ae7d4 commit bfea422
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
59 changes: 59 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
3 changes: 2 additions & 1 deletion docs/spec_checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit bfea422

Please sign in to comment.