-
Notifications
You must be signed in to change notification settings - Fork 159
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add initial contributing and code style guides (#131)
These are intended to deal with much of the low-hanging fruit; plenty of room for improvement exists.
- Loading branch information
Showing
2 changed files
with
110 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
Code Style | ||
========== | ||
|
||
This document is an incomplete description of the predominant style used in the | ||
RISC-V Sail model. | ||
Where something is not specified, please look for existing code similar to what | ||
is being added and copy the predominant style. | ||
|
||
For C and OCaml, the formatting rules should be followed where applicable. | ||
For other languages, follow the standard style for that language if it exists; | ||
for example, Python should follow the standard PEP-8 style. | ||
|
||
Formatting | ||
---------- | ||
|
||
* Block-level indentation uses two spaces | ||
|
||
* There should be no trailing spaces on any lines | ||
|
||
* All files should end with a newline character | ||
|
||
* Unix-style line endings should be used | ||
|
||
* Files should be free from leading, trailing and double blank lines | ||
|
||
* There should be one space either side of operators such as `=` and `+` | ||
|
||
* There should be no spaces before and one space after `,` | ||
|
||
* There should be one space after control flow keywords such as `if`, `foreach` | ||
and `match` | ||
|
||
* There should be no spaces between a function name and its arguments, for both | ||
definitions (valspecs, function definitions and function clauses) and calls, | ||
nor should there be any spaces immediately within the parentheses | ||
|
||
* There should be no spaces between a vector and the opening square bracket for | ||
indexing or slicing, nor should there be any spaces immediately within the | ||
square brackets | ||
|
||
* For large blocks of code with repetitive structure where it improves | ||
readability, additional whitespace may be inserted to align corresponding | ||
elements horizontally with each other, within reason | ||
|
||
* Avoid unnecessary parentheses and curly braces unless doing so seriously | ||
hurts readability | ||
|
||
* When modifying existing code that does not conform to this style, prefer | ||
matching the existing style | ||
|
||
Implementation | ||
-------------- | ||
|
||
* Since this is the official model intended to be included as part of the | ||
RISC-V specifications, readability is paramount, including to those not | ||
already familiar with the details of that field of Computer Science (e.g. | ||
floating-point or cryptography) | ||
|
||
* All instructions should be built as part of both the RV32 and RV64 models so | ||
as to provide a path to supporting mutable MXL/SXL/UXL; if necessary, | ||
constructs like `assert(sizeof(xlen) == 32)` at the start of the body can be | ||
used to suppress any type errors that arise as a result | ||
|
||
* Avoid the use of hard-coded constants like 32 even if the instruction is | ||
RV32-specific, instead favouring `sizeof(xlen)` or a computed constant to | ||
more clearly express the underlying intent | ||
|
||
* Local variables should be made immutable whenever possible, but short | ||
imperative loops with a small amount of local mutable state are preferred | ||
over less-readable functional-style recursive equivalents | ||
|
||
* Choose carefully between integer and bitvector types, and avoid multiple | ||
round-trips between the two; for example, if counting something, use a `nat` | ||
and convert it to an appropriately-sized bitvector at the point it is stored | ||
in a register, but keep register source values as bitvectors until they are | ||
needed to be interpreted as integers (see the implementation of `MUL` as an | ||
example) | ||
|
||
* Prefer `bool` over `bits(1)` when a value logically represents true or false | ||
rather than being a single bit with a numeric meaning, and vice-versa | ||
|
||
* No new compile-time warnings from the Sail compiler should be introduced | ||
(this does not include C or OCaml warnings for the code generated by the Sail | ||
compiler) | ||
|
||
* Do not use the `ext*` types and hooks for standard extensions unless | ||
providing a stub implementation; these are reserved for use by out-of-tree | ||
extensions that provide their own non-stub implementations |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
Contributing | ||
============ | ||
|
||
When contributing, please follow the [code style](CODE_STYLE.md) in use. | ||
|
||
Pull requests should be a single set of related changes with a clean commit | ||
history free from merge commits. | ||
Each commit should be self-contained, provide a meaningful short summary in | ||
the subject and, if not clear from the subject alone, provide more detail in | ||
the commit description. | ||
|
||
Every commit should build with no new regressions introduced, and large commits | ||
should be broken up into multiple distinct commits that take incremental steps | ||
towards the final goal. | ||
For example, large ratification packages should have one commit per individual | ||
extension, with possibly an additional initial commit to add necessary | ||
infrastructure like new types needed for all the subsequent commits. | ||
|
||
Unnecessary code churn should be avoided unless as part of a pull request aimed | ||
at improving code quality, such as fixing repeated code style violations or | ||
renaming a function whose meaning is unclear. | ||
Such pull requests should not also introduce significant new functionality. |