-
-
Notifications
You must be signed in to change notification settings - Fork 1
Contributing
- Fork the repository
- Create a feature branch from
developwith a descriptive name - Make your changes, ensuring existing tests still pass
- Add new tests covering your changes
- Run
ctestto verify all tests pass - Open a pull request targeting the
developbranch
nsc_core and mips_core must never include headers from nsc_ui or nsc_qt. This constraint is enforced via CMake target link dependencies. If you need the UI to know something about the core, expose it through IProcessor or PipelineState — do not reach back from the core into the UI.
Use enum class for hardware fields rather than plain integers. This prevents silent errors when passing opcode values where funct values are expected (or vice versa). Existing enums:
-
AluOp— ALU operation selector -
StepResult— CPU step outcome (Continue,Halt,Error) - Instruction format types
Use std::optional and exceptions for operations that can fail at runtime:
-
Decoder::decode()returnsstd::optional<Instruction>for unknown opcodes - Memory alignment errors throw rather than silently truncating
Do not return sentinel integer values (-1, 0xDEADBEEF) to signal errors in functions that could plausibly return those values legitimately.
Apply const aggressively on member functions that do not mutate state, especially on IProcessor query methods:
virtual PipelineState pipeline_state() const = 0;
virtual uint32_t read_register(int reg) const = 0;Apply [[nodiscard]] on functions whose return value must not be silently discarded — in particular, the ALU result and decoder output.
A grep for #include "ftxui/ or #include <QWidget> in src/mips/ or src/nsc/ should return nothing. CI should enforce this.
Tests live in tests/ mirroring the src/ structure:
tests/
mips/
decoder_test.cpp
cpu_test.cpp
processor_test.cpp ← polymorphic: runs both backends
nsc/
nsc_tests.cpp
qt/
qt_ui_test.cpp
Target individual components (parseBase, ALU functions, individual decoder cases). Keep each test focused on a single behavior. Use ASSERT_EQ / EXPECT_EQ from GoogleTest.
processor_test.cpp runs the same programs through both SingleCycleCpu and PipelinedCpu and asserts identical final register and memory state. Any new CPU feature should be exercised through this harness so behavioral parity is maintained.
When adding a new instruction to the ISA:
- Add a decoder case in
Decoder::decode() - Add an ALU or memory path in both
SingleCycleCpuandPipelinedCpu - Add a test program in
processor_test.cppthat exercises the instruction in isolation and in a sequence that would trigger hazards
qt_ui_test.cpp exercises the SimulatorController signal/slot wiring without a visible window. These tests are skipped when BUILD_QT6_UI=OFF. Keep them lightweight — they guard signal connectivity, not simulation correctness (that is covered by processor_test).
- All new
.cppfiles must be added to the appropriatetarget_sources()call inCMakeLists.txt. Forgetting this produces a linker error, not a compile error, so it can be confusing. - Qt's MOC requires that
Q_OBJECTclasses be listed in CMake — they are not discovered automatically. - FTXUI is fetched by CMake at configure time (
FetchContent). Do not vendor it manually.
Follow the project's existing style (visible in git log):
- Imperative mood: "Add forwarding unit" not "Added forwarding unit"
- First line ≤ 72 characters
- Body (if needed) explains why, not what — the diff shows what changed
If you're unsure whether a change fits the project's direction, open an issue before writing code. Questions about code conventions are welcome — the architecture is intentionally academic, and the design decisions are worth discussing.