Skip to content

Comments

feat: support scalar kernel params in runner and test infra#33

Merged
tetsuo-cpp merged 6 commits intocanonfrom
feat/scalar-params
Feb 19, 2026
Merged

feat: support scalar kernel params in runner and test infra#33
tetsuo-cpp merged 6 commits intocanonfrom
feat/scalar-params

Conversation

@tetsuo-cpp
Copy link
Owner

Summary

  • Add typed --param format to warpforth-runner: i64:42 (scalar) and i64[]:1,2,3 (array)
  • Update conftest.py to parse scalar param declarations and pass them with the typed format
  • Add pipeline test (scalar-param.forth) and GPU test (test_scalar_param) for mixed scalar+array kernels

Closes #32

Copy link
Owner Author

@tetsuo-cpp tetsuo-cpp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Clean PR — scalar param support is well-structured across both the C++ runner and Python test infra. The ParamDecl dataclass is a nice improvement over the raw tuple. A few items to consider:

Actionable

  1. Empty value handling in parseParam (warpforth-runner.cpp): Passing --param i64: or --param i64[]: will hit std::stoll on an empty string and throw an uncaught exception. Worth validating that valueStr is non-empty (and that at least one value was parsed for arrays).

  2. Pointer-lifetime comment (warpforth-runner.cpp:~214): scalarValues exists so kernelArgs[i] can point at &scalarValues[i] across the kernel launch. This is correct but non-obvious — a brief comment would help future readers.

  3. Out-of-bounds output_param (conftest.py:387): The guard output_param < len(decls) silently skips the scalar check when the index is out of bounds, deferring the error to a less obvious place. Consider failing early on out-of-bounds too.

Optional / Nits

  • C-style casts in the kernel arg ternary — consistent with the rest of the file but static_cast would be more idiomatic C++17.
  • No negative tests for the runner's error paths (bad type prefix, scalar with multiple values, scalar as output). A quick lit test checking for the expected stderr would be nice coverage.
  • The MID FileCheck line is sensitive to MLIR attribute ordering — could become flaky if attribute print order changes upstream.

Overall looks good 👍

@tetsuo-cpp tetsuo-cpp merged commit 64f90a9 into canon Feb 19, 2026
1 check passed
@tetsuo-cpp tetsuo-cpp deleted the feat/scalar-params branch February 19, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support scalar params in warpforth-runner

1 participant