Skip to content

feat(rocke): prototype device-side printing - #10841

Draft
tenpercent wants to merge 14 commits into
developfrom
users/mpodkory/rocke/aick-1989-device-print
Draft

tenpercent wants to merge 14 commits into
developfrom
users/mpodkory/rocke/aick-1989-device-print

Conversation

@tenpercent

@tenpercent tenpercent commented Aug 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • add a checked gpu.device_print Text/Value record to the Python and C/C++ rocKE engines
  • support bool, i32, u32, f32, and global-pointer values
  • lower one NUL-terminated record through OCKL, chaining values in groups of at most seven
  • request COMGR device libraries only for kernels that print, including nested regions
  • add mirrored verification, serialization, LLVM parity, and GPU runtime coverage

ISSUE ID : AICK-1989

Behavior

  • bool values print as t or f on the ordinary OCKL argument path
  • Python Text rejects embedded NUL; C Text is a NUL-terminated ASCII string
  • ROCKE_ENGINE_DEVICE_PRINT_MAX_VALUE_COUNT controls the logical record limit;
    the seven-value OCKL packet width is fixed

Integration

Refreshed against develop e1e7fdcab9be0f79256e47d653808c7f2e6e1948, resolving
the four conflicts while retaining current compiler AUTO policy, target resolution,
nontemporal metadata, and native test registration. The new opcode is appended so
all 198 existing opcode values remain unchanged. The installed native print test
is registered, and architecture-domain probes omit external OCKL declarations.

Validation

Validated repair head: c4044a3cf2ecdd00c78de234fca8d7bb64842cea.

  • Fresh native archive and Python extension build; 17/17 native CTests passed.
  • Broad host suite: 1,222 passed, 67 skipped, 3,762 subtests passed.
  • Final focused suite: 56 passed, 3,590 subtests passed, including the ordinary,
    print, and nested-print compile-link flag matrix through Python, strict C++, and both.
  • Python/C++ LLVM byte identity: all 77 families / 960 configurations per
    flavor green under LLVM20, LLVM22, and LLVM23.
  • Device-print canonical records: three configurations green; verifier
    differential: three configurations green at each of the three LLVM flavors.
  • Relocated installed artifact: two native CTests and 23 Python tests passed.
  • gfx950 COMGR compilation and GPU output tests passed through Python, strict
    C++, and both backends, with ROCm 7.14.0 and queried LLVM 23.0.0. Covered false
    predicate silence, all supported value types, and eight values spanning the
    second OCKL packet. The GPU payload used source tree
    3bc68dc109cba5f19ddcce6aa73a5b4764ca1bfd; only host test-matrix additions
    changed under rocKE afterward.
  • Required pre-commit hooks over the final PR range and git diff --check passed.
  • Required COMGR device-library setter signature and symbol checked on ROCm 7.1.

Validation limits: the full ArchDomainRegenerationTest sweep was excluded; other
architecture-domain artifact checks passed. The relative-path guard flags
cmake/InstallGpuReferences.cmake:30, which is byte-identical to develop.
GPU runtime coverage is limited to gfx950. The relocated artifact predates the
final host test-matrix additions; its production code matches the repaired head.

Scope

This PR covers the checked record, OCKL lowering, and the supported value types
listed above. Typed output, additional dtypes, non-global pointers, vectors,
packed formats, presentation controls, higher-level helpers, and alternative
sinks remain follow-up work.

@tenpercent
tenpercent force-pushed the users/mpodkory/rocke/aick-1989-device-print branch from 0d9c132 to 6c85182 Compare August 15, 2026 00:25
@therock-pr-bot

therock-pr-bot Bot commented Aug 15, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@tenpercent tenpercent changed the title feat(hip-kernel-provider): prototype device-side printing feat(rocke): prototype device-side printing Aug 15, 2026
@tenpercent
tenpercent marked this pull request as ready for review August 15, 2026 06:20
@tenpercent
tenpercent requested review from a team as code owners August 15, 2026 06:20
@tenpercent
tenpercent requested a review from a team August 15, 2026 06:26
@tenpercent
tenpercent marked this pull request as draft August 17, 2026 16:58
@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Sep 12, 2026
@tenpercent

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

@tenpercent tenpercent removed the Stale PR has no activity for 25+ days label Sep 12, 2026
…ke/aick-1989-device-print

# Conflicts:
#	dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/ir.py
#	dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/core/lower_llvm.py
#	dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/helpers/compile.py

Co-authored-by: tenpercent <4273004+tenpercent@users.noreply.github.com>

Copilot AI commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Conflicts are resolved by merge commit f511089d4 (merged origin/develop and resolved the three rocke Python conflict files).

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Oct 8, 2026
@cgmillette

cgmillette commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Hi @tenpercent , recommend to update the PR or close

PR #10841 — device-side printing prototype (tenpercent, AICK-1989) — 🟠 REJECT: stale draft; rebase or close

Prior reviews: none (bots: stale ×2; Copilot-agent merge f511089d, unreviewed).

# Sev file:line Finding
N1 🟠 PR state Draft, Stale (auto-close pending), 768 behind, 4 conflicts
N2 🟠 comgr.py, compile.py, lower_llvm.py Re-port onto #12678 / #12783 / #12672; all validation predates them

Utility: 🟢 Sound concept — mirrored printf; % escaping and f32→f64 promotion verified; revive or close.


Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good

@github-actions github-actions Bot removed the Stale PR has no activity for 25+ days label Oct 10, 2026
Preserve compiler selection, LLVM layouts and temporal hints; append the new opcode without renumbering existing C values. Install the native test and cover device-library linking across lowering backends. Keep external OCKL functions outside the compiler intrinsic-domain probe.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Ambient validation knobs, malformed-input normalization, and incomplete COMGR flag propagation leave supported compilation paths inconsistent.

5 open findings
What changed in this PR

Adds cross-engine device-side printing to rocKE, including validated IR records, OCKL lowering, conditional device-library linking, and runtime coverage.

Changes:

  • Adds Python and C/C++ device-print APIs, validation, serialization, and LLVM lowering.
  • Links ROCm device libraries when printing is detected.
  • Adds parity, host, GPU, architecture-domain, and installation tests.
File Description
platform/​tools/​gen_arch_domain.py Excludes device-library declarations from intrinsic probes.
platform/​tests/​runtime/​test_device_print_gpu.py Validates GPU print output.
platform/​tests/​runtime/​device_print_gpu_runner.py Builds and launches the print test kernel.
platform/​tests/​README.md Documents runtime coverage.
platform/​tests/​instances/​parity/​device_print_emit.py Adds Python parity emitter.
platform/​tests/​instances/​parity/​device_print_emit.c Adds C parity emitter.
platform/​tests/​core/​test_device_print.py Tests Python construction, verification, lowering, and linking.
platform/​tests/​core/​test_arch_domain_artifact.py Tests exclusion of OCKL declarations.
platform/​tests/​core/​device_print.cpp Tests the native C API.
platform/​tests/​CMakeLists.txt Registers and installs the native test.
platform/​python/​rocke/​runtime/​comgr.py Adds optional device-library linking.
platform/​python/​rocke/​helpers/​compile.py Detects printing during compilation.
platform/​python/​rocke/​core/​verify.py Verifies canonical print records.
platform/​python/​rocke/​core/​lower_llvm.py Lowers printing through OCKL.
platform/​python/​rocke/​core/​ir.py Adds print records and builder API.
platform/​python/​rocke/​core/​__init__.py Exports print record types.
platform/​python/​rocke/​__init__.py Exports print record types publicly.
platform/​dsl_docs/​reference/​env_flags.md Documents print limits.
platform/​cpp/​include/​rocke/​lower_llvm_internal.h Adds native lowering state.
platform/​cpp/​include/​rocke/​ir.h Adds the C print API and opcode.
platform/​cpp/​core/​verify.cpp Adds native record verification.
platform/​cpp/​core/​lower_llvm/​device_print.cpp Implements native OCKL lowering.
platform/​cpp/​core/​lower_llvm/​data.cpp Registers OCKL declarations.
platform/​cpp/​core/​lower_llvm/​core.cpp Registers lowering and emits format globals.
platform/​cpp/​core/​ir/​device_print.cpp Implements native record construction.
platform/​cpp/​core/​ir/​device_print_internal.h Declares internal validation helpers.
platform/​cpp/​core/​ir/​core_types.cpp Registers opcode name and purity.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

llvm_text,
isa=isa,
options=_comgr_options_for_kernel(kernel),
link_device_libraries=_kernel_requires_device_libraries(kernel),
static int parse_device_print_limit(
const char* name, int default_value, int* value, char* error, size_t error_capacity)
{
const char* raw = getenv(name);
Comment on lines +158 to +159
style = style ? style : "compact";
termination = termination ? termination : "ensure_newline";


def _device_print_limit(name: str, default: int) -> int:
raw = os.environ.get(name)
raise TypeError(
"device_print Value item must contain a rocke SSA Value"
)
logical = item.format or _default_print_format(item.value.type)
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

  • Add new commits
  • Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

@github-actions github-actions Bot added the Stale PR has no activity for 25+ days label Nov 5, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants