Skip to content

Commit 9abee3f

Browse files
LessUpCopilot
andcommitted
openspec: fix review issues in performance-teaching-hardening change
- design.md: replace INTERFACE library guidance with simd_dispatch STATIC library target; simd_utils remains header-only - design.md: add explicit GCC/Clang guard for __builtin_cpu_supports; document MSVC scalar-fallback constraint - design.md: remove examples/02-memory-cache/README.md as benchmark regression docs target; keep benchmarks/README.md only - design.md: resolve docs/ placeholders to exact paths (docs/en/guides/learning-path.md, docs/en/guides/validation.md) - design.md: add Mermaid architecture/file-flow diagram - tasks.md: align with design changes (STATIC lib, exact doc paths) - specs/benchmark-framework/spec.md: absorb smoke-test scenario from ci-quality-assurance; add threshold-configurable scenario - specs/ci-quality-assurance/spec.md: remove Benchmark Regression Script Testable requirement (owned by benchmark-framework) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 53edff4 commit 9abee3f

4 files changed

Lines changed: 35 additions & 20 deletions

File tree

openspec/changes/performance-teaching-hardening/design.md

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,35 @@
44

55
This change closes four teaching gaps on existing surfaces without introducing new modules. All work is bounded to `examples/04-simd-vectorization/`, `scripts/`, `docs/`, and module README files.
66

7+
## Architecture overview
8+
9+
```mermaid
10+
graph TD
11+
A[runtime_dispatch.cpp] -->|compiled into| B[simd_dispatch STATIC lib]
12+
C[simd_utils INTERFACE lib<br/>header-only] -->|provides headers| B
13+
B -->|linked by| D[dispatch_example executable]
14+
B -->|linked by| E[simd_dispatch_test]
15+
F[scripts/compare_benchmarks.py] -->|reads| G[baseline.json]
16+
F -->|reads| H[candidate.json]
17+
F -->|writes| I[regression table + exit code]
18+
J[docs/en/guides/learning-path.md] -->|links to| K[examples/04-simd-vectorization/README.md]
19+
J -->|links to| L[docs/en/guides/validation.md]
20+
M[README.md] -->|cross-links| L
21+
N[benchmarks/README.md] -->|documents| F
22+
```
23+
724
## Design decisions
825

926
### 1. SIMD runtime dispatch
1027

1128
**Goal**: Show readers how to select the fastest available instruction path at runtime rather than at compile time.
1229

13-
**Approach**: Add `examples/04-simd-vectorization/src/runtime_dispatch.cpp` with a `dispatch_add_arrays` function that uses `cpuid` (via `__builtin_cpu_supports` on GCC/Clang) to select AVX2, SSE2, or scalar at runtime. Export the function through the existing `simd_utils` interface library so the existing test runner can reach it.
30+
**Approach**: Add `examples/04-simd-vectorization/src/runtime_dispatch.cpp` with a `dispatch_add_arrays` function that uses `cpuid` (via `__builtin_cpu_supports` on GCC/Clang) to select AVX2, SSE2, or scalar at runtime. The function is compiled into a new **`STATIC` library target `simd_dispatch`** (not the existing `simd_utils` INTERFACE target) that links `simd_utils` for headers. The example executable and the corresponding test both link `simd_dispatch`. `simd_utils` remains header-only.
1431

1532
**Rationale**: `__builtin_cpu_supports` is available on GCC ≥ 4.8 and Clang ≥ 3.7, covers the C++17 baseline, and avoids a platform-specific CPUID wrapper. The function name stays within the `hpc::simd` namespace. A companion `tests/` entry validates correctness against the scalar reference.
1633

34+
**Compiler guard**: `__builtin_cpu_supports` is a GCC/Clang extension. The implementation must wrap dispatch logic in `#if defined(__GNUC__) || defined(__clang__)`. On any other compiler (e.g., MSVC) the code falls through to the scalar path unconditionally. MSVC-specific CPUID dispatch is explicitly out of scope for this change.
35+
1736
**Trade-off**: Does not use `ifunc` or a separate DSO; runtime dispatch is done once via a function pointer set at call site. This is simpler and sufficient for a teaching example.
1837

1938
### 2. Vectorization diagnostics workflow
@@ -36,7 +55,7 @@ This change closes four teaching gaps on existing surfaces without introducing n
3655

3756
**Goal**: A maintainer can compare two benchmark JSON runs and see which benchmarks regressed.
3857

39-
**Approach**: Add `scripts/compare_benchmarks.py` (Python 3, stdlib only — no third-party packages) that accepts two JSON files (baseline and candidate) and prints a table of benchmark name, baseline ns/iter, candidate ns/iter, and delta %. Exit code 1 if any benchmark regresses by more than a configurable threshold (default 10%). Add a "Regression Comparison" section to `examples/02-memory-cache/README.md` and the relevant benchmark docs entry showing the capture-and-compare workflow.
58+
**Approach**: Add `scripts/compare_benchmarks.py` (Python 3, stdlib only — no third-party packages) that accepts two JSON files (baseline and candidate) and prints a table of benchmark name, baseline ns/iter, candidate ns/iter, and delta %. Exit code 1 if any benchmark regresses by more than a configurable threshold (default 10%). Add a "Regression Comparison" section to `benchmarks/README.md` showing the capture-and-compare workflow.
4059

4160
**Rationale**: stdlib-only ensures the script works without a virtualenv. The threshold flag makes it usable in CI without hardcoding expected values.
4261

@@ -47,11 +66,11 @@ This change closes four teaching gaps on existing surfaces without introducing n
4766
| Path | Change |
4867
|------|--------|
4968
| `examples/04-simd-vectorization/src/runtime_dispatch.cpp` | New: runtime CPU dispatch example |
50-
| `examples/04-simd-vectorization/CMakeLists.txt` | Extend: wire `runtime_dispatch` target |
69+
| `examples/04-simd-vectorization/CMakeLists.txt` | Extend: add `simd_dispatch` STATIC library target |
5170
| `tests/` (simd subdir) | New: correctness test for `dispatch_add_arrays` |
5271
| `examples/04-simd-vectorization/README.md` | Extend: vectorization diagnostics section |
53-
| `docs/` (SIMD learning path entry) | Extend: vectorization diagnostics, sanitizer link |
54-
| `docs/` (validation/safety page or section) | New or extend: sanitizer preset workflow |
72+
| `docs/en/guides/learning-path.md` | Extend: vectorization diagnostics, sanitizer cross-link |
73+
| `docs/en/guides/validation.md` | New: sanitizer preset workflow (asan/tsan/ubsan) |
5574
| `README.md` | Extend: cross-link to sanitizer docs |
5675
| `scripts/compare_benchmarks.py` | New: benchmark regression comparison script |
57-
| `benchmarks/` README or docs entry | Extend: capture-and-compare workflow |
76+
| `benchmarks/README.md` | New: capture-and-compare workflow section |

openspec/changes/performance-teaching-hardening/specs/benchmark-framework/spec.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,8 @@ THE HPC_Guide SHALL provide a script to compare two Google Benchmark JSON output
2020

2121
- **WHEN** the script is invoked with `--threshold N`
2222
- **THEN** the regression threshold is set to N percent rather than the default 10 percent
23+
24+
#### Scenario: Script smoke test passes
25+
26+
- **WHEN** `scripts/compare_benchmarks.py` is invoked with two synthesised JSON inputs (one stable, one regressed)
27+
- **THEN** it exits 0 for the stable case and exits 1 for the regressed case, confirming the script is functional
Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,3 @@
11
# CI and Quality Assurance
22

3-
## ADDED Requirements
4-
5-
### Requirement: Benchmark Regression Script Testable
6-
7-
THE Build_System SHALL allow the benchmark regression comparison script to be smoke-tested without a full benchmark run.
8-
9-
#### Scenario: Script smoke test passes
10-
11-
- **WHEN** `scripts/compare_benchmarks.py` is invoked with two synthesised JSON inputs (one stable, one regressed)
12-
- **THEN** it exits 0 for the stable case and exits 1 for the regressed case, confirming the script is functional
3+
No new requirements added in this change. The benchmark regression script smoke-test requirement is owned by the `benchmark-framework` capability.

openspec/changes/performance-teaching-hardening/tasks.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,22 @@
33
## 1. SIMD runtime dispatch
44

55
- [ ] 1.1 Add `examples/04-simd-vectorization/src/runtime_dispatch.cpp` with `hpc::simd::dispatch_add_arrays` using `__builtin_cpu_supports` for AVX2/SSE2/scalar selection
6-
- [ ] 1.2 Register `runtime_dispatch` target in `examples/04-simd-vectorization/CMakeLists.txt` via `hpc_add_example`
6+
- [ ] 1.2 Add a `simd_dispatch` STATIC library target in `examples/04-simd-vectorization/CMakeLists.txt` (separate from the INTERFACE `simd_utils` target) that compiles `runtime_dispatch.cpp` and links `simd_utils` for headers
77
- [ ] 1.3 Add a correctness test under `tests/` that calls `dispatch_add_arrays` and validates results against the scalar reference
88
- [ ] 1.4 Verify `cmake --preset=debug && cmake --build build/debug && ctest --preset=debug` passes with the new target and test
99

1010
## 2. Vectorization diagnostics documentation
1111

1212
- [ ] 2.1 Add a "Vectorization Diagnostics" section to `examples/04-simd-vectorization/README.md` with GCC (`-fopt-info-vec`) and Clang (`-Rpass=loop-vectorize`) flag examples and sample output
13-
- [ ] 2.2 Add or extend a docs site page for the SIMD module to surface the vectorization diagnostics workflow for readers
13+
- [ ] 2.2 Extend `docs/en/guides/learning-path.md` (SIMD section) to surface the vectorization diagnostics workflow for readers
1414

1515
## 3. Sanitizer workflow documentation
1616

17-
- [ ] 3.1 Add a "Validation and Safety" section to the VitePress docs site documenting the `asan`, `tsan`, and `ubsan` presets with copy-pasteable commands
17+
- [ ] 3.1 Add `docs/en/guides/validation.md` documenting the `asan`, `tsan`, and `ubsan` presets with copy-pasteable commands; add entry to VitePress nav
1818
- [ ] 3.2 Cross-link the sanitizer section from the root `README.md` quick-start
1919

2020
## 4. Benchmark regression comparison
2121

2222
- [ ] 4.1 Add `scripts/compare_benchmarks.py`: accepts two Google Benchmark JSON files, prints a regression table (name, baseline, candidate, delta%), exits 1 if any benchmark exceeds the threshold (default 10%, configurable via `--threshold`)
23-
- [ ] 4.2 Add a "Regression Comparison" section to the benchmarks docs entry or `benchmarks/` README showing the capture-and-compare workflow
23+
- [ ] 4.2 Add a "Regression Comparison" section to `benchmarks/README.md` showing the capture-and-compare workflow
2424
- [ ] 4.3 Smoke-test the script with two synthesised JSON inputs to confirm it exits 0 on stable and 1 on a regressed run

0 commit comments

Comments
 (0)