Skip to content

feat(hipsr): add compute and compute_yield ops - #614

Closed
zpye wants to merge 1 commit into
ROCm:mainfrom
zpye:feat/hipsr-compute-op
Closed

feat(hipsr): add compute and compute_yield ops#614
zpye wants to merge 1 commit into
ROCm:mainfrom
zpye:feat/hipsr-compute-op

Conversation

@zpye

@zpye zpye commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds hipsr.compute and hipsr.compute_yield. hipsr.compute is a DPS op whose isolated single-block region groups the operations that implement one high-level computation, so an ONNX op needing several tensor or shape operations stays a single hipsr op.

Ops only: nothing emits one yet, so no existing behavior changes.

Related issue or design

None.

Why

hipsr.placeholder accepts only block arguments and constant or placeholder results as inputs. Without a grouping op, a multi-operation lowering leaves intermediates in the enclosing block, and the placeholder of a later op would have to take one as an input, which its verifier rejects.

The body is IsolatedFromAbove, so the context, inputs, and inits enter it as entry-block arguments in that order.

There is no hand-written verifier. SizedRegion<1>, SingleBlock, SingleBlockImplicitTerminator, RegionBranchOpInterface, DestinationStyleOpInterface, and IsolatedFromAbove already produce every check, and the tests assert the message each one emits.

What

  • hipsr.compute: !hipsr.context first, then variadic ins and outs inits, variadic tensor results, one $body region.
  • Inits are Hipsr_TensorOrDeviceMemRef, so the op survives bufferization to device memrefs. Memref inits mean zero tensor results, which the optional result-type group allows.
  • Inputs are Variadic<AnyType>, so the body can also take values like an index extent.
  • hipsr.compute_yield mirrors hipsr.pool_domain_yield.
  • No shape region: output shapes stay the hipsr.placeholder job, and -hipsr-populate-shape-region skips this op.

Test plan

New test/lit/Dialect/Hipsr/compute.mlir: three positive cases (tensor init, multiple inits with a non-tensor input, device-memref inits with no results) and ten negative cases, one per verification mechanism. Expected diagnostics were copied from real hip-mlir-opt output.

ctest -R MorphizenMLIRLitTests
366 tests: 361 passed, 3 unsupported, 2 failed

Both failures are pre-existing on main and unrelated: test_gather_block_quantized.mlir (--convert-onnx-to-hip) and test_compress_model.mlir (--hipdnn-pipeline). pre-commit run --all-files passes.

Notes for reviewers

  • No bufferization model, so a hipsr.compute reaching one-shot bufferize would fail. Unreachable today because -hipsr-bufferize does not exist.
  • No custom builder. TableGen's (resultTypes, ctx, inputs, inits) builder leaves the body blockless, so the first conversion pattern must create the entry block itself; @empty_body and @missing_context_argument pin the diagnostics it sees otherwise.
  • Nothing computes output shapes for compute-backed ONNX ops yet. A follow-up must decide where reshape-like shape math lives.

Checklist

  • The change is focused, or links a design/series explaining its scope.
  • Relevant tests were added or updated and the results are documented.
  • User-facing or design documentation was updated when needed: no in-repo doc covers hipsr, so the ODS description blocks carry the contract.
  • Substantial AI assistance is disclosed, and I reviewed and understand the result.

AI assistance: drafted with Cursor. Diagnostics were captured from real tool output and the test numbers come from local build and CTest runs. The commit carries Made-with and Co-authored-by trailers.

hipsr.compute groups the operations that implement one high-level
computation into a single DPS op with an isolated body region, so an ONNX
op needing several tensor or shape operations does not leave intermediate
values in the enclosing block. A hipsr.placeholder accepts only block
arguments and constant or placeholder results as inputs, so loose
intermediates would otherwise block placeholder-based output shaping.

The op carries no hand-written verifier. Every rule the design proposed
checking by hand already comes from a trait or interface: SizedRegion<1>
and SingleBlock for the block count, SingleBlockImplicitTerminator for
the terminator, RegionBranchOpInterface for both control-flow edges,
DestinationStyleOpInterface for init and result tying, and
IsolatedFromAbove for boundary crossings. test/lit/Dialect/Hipsr/
compute.mlir asserts the message each mechanism emits.

Two traits from the design are left out on purpose.
PromotableRegionOpInterface does not exist in the pinned MLIR, and the
Promotable* family it resembles is Mem2Reg/SROA slot promotion rather
than region flattening. ConditionallySpeculatable only feeds isPure,
whose consumers are LICM and related hoisting paths; the hipsr pipeline
has no loops to hoist a compute out of, and RecursiveMemoryEffects
already lets dead-code elimination erase an unused one.

This step adds the ops only, with no conversion-pattern users and no
bufferization model, so a compute that reached one-shot bufferize would
fail. That is unreachable today because -hipsr-bufferize does not exist.

Made-with: Cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown

Thanks for opening a PR!

This project follows LLVM's incremental-development and AI-tool-use
guidance. See CONTRIBUTING.md
for the project workflow.

Before requesting review, please check that:

  1. The change is focused. Substantial work links the relevant issue
    or design discussion.
  2. The PR documents relevant test results and updates affected
    documentation.
  3. If AI tools provided substantial assistance, the description
    explains what was assisted and how it was validated, and commit
    trailers identify the tool. The contributor has reviewed and
    understands the result.

Reviewers are assigned through
CODEOWNERS where ownership
is configured.

@zpye
zpye marked this pull request as ready for review July 31, 2026 11:30
@zpye zpye closed this Jul 31, 2026
@zpye

zpye commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #615, which carries the same commit from a branch in this repository so that CI runs with full repository access. Closing this one.

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.

1 participant