fix(codegen): wrap scalar cacheinvalid in make_tensor_view for ptoas 0.50 by georgebisbas · Pull Request #2137 · hw-native-sys/pypto

georgebisbas · 2026-07-24T09:53:27Z

The scalar-write branch of system.cacheinvalid codegen in pto_ops_memory.cpp had two PTOAS 0.50 incompatibilities:

Missing view operand: emitted pto.cmo.cacheinvalid against a raw !pto.ptr<T> without a type annotation. PTOAS 0.50+ requires a typed tensor_view or partition_view operand.
Raw integer literals in make_tensor_view: shape = [1], strides = [1] used bare 1 literals instead of SSA index constants (%c1_index), which PTOAS 0.50 cannot parse.

Fix (src/backend/common/pto_ops_memory.cpp):

Extracted EmitScalarTensorViewPTO helper that consolidates the addptr + make_tensor_view(1xT) 3-step pattern for 1-element scalar views. Both scalar and partition-view branches now compute (view_ssa, view_type) and converge to a single shared pto.cmo.cacheinvalid emit — future PTOAS syntax changes only need one emit site update.

auto [view_ssa, view_type] = EmitScalarTensorViewPTO(base_ptr, off, ptr_type, dtype_str, codegen);
// ...
codegen.Emit("pto.cmo.cacheinvalid " + view_ssa + " single_cache_line : " + view_type);

Generated PTO:

%0 = pto.addptr %arg2, %c0_index : !pto.ptr<f32> -> !pto.ptr<f32>
%1 = pto.make_tensor_view %0, shape = [%c1_index], strides = [%c1_index] {layout = #pto.layout<nd>} : !pto.tensor_view<1xf32>
pto.cmo.cacheinvalid %1 single_cache_line : !pto.tensor_view<1xf32>

Test (tests/ut/codegen/test_pto_codegen_ops.py):

Strengthened test_cacheinvalid_scalar_write_emits_scalar_view (renamed from _ptr) to assert pto.make_tensor_view presence, typed view operand (: !pto.tensor_view<1x...>), and SSA index constants (shape = [%c1_index], strides = [%c1_index]) — catching both regressions that the two PTOAS 0.50 incompatibilities introduced.

The bug only triggers when valid_shape=[1, 1] (size=1 tests), e.g. test_l3_tensor_allreduce_intrinsic[2-1].

Fixes: #2136

coderabbitai · 2026-07-24T09:54:26Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1098b89e-ab0d-45fd-936f-a86bb6d7d538

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The scalar system.cacheinvalid lowering now creates a typed 1-element tensor view from the computed write pointer and uses that view for pto.cmo.cacheinvalid.

Changes

Scalar cache invalidation

Layer / File(s)	Summary
Typed scalar view lowering `src/backend/common/pto_ops_memory.cpp`	The scalar-write branch creates a `!pto.tensor_view` with shape and stride `[1]`, then emits `pto.cmo.cacheinvalid` using the view.

Estimated code review effort: 2 (Simple) | ~5 minutes

Possibly related PRs

hw-native-sys/pypto#2060: Added the system.cacheinvalid lowering that this scalar-path fix refines.

Poem

A bunny found a pointer bare,
And wrapped it in a view with care.
One little tile, typed just right,
Now cache invalidation takes flight.
Hop, hop—compiled tonight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change matches `#2136` by wrapping scalar cacheinvalid in make_tensor_view and adding an explicit view type.
Out of Scope Changes check	✅ Passed	The PR stays focused on the scalar cacheinvalid codegen fix with no unrelated changes.
Title check	✅ Passed	The title clearly summarizes the scalar cacheinvalid tensor-view fix for PTOAS 0.50.
Description check	✅ Passed	The description matches the code and test changes and explains the PTOAS 0.50 compatibility fix.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

src/backend/common/pto_ops_memory.cpp (1)
861-865: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add assertions for the new typed scalar view.

The scalar regression test currently verifies only single_cache_line and the absence of partition_tensor_view; it would also pass with the old raw-pointer emission. Assert that the generated IR contains pto.make_tensor_view with [1]/[1] and that cacheinvalid includes : !pto.tensor_view<1x...> so this fix is protected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/common/pto_ops_memory.cpp` around lines 861 - 865, Add
regression-test assertions for the scalar view emitted around `scalar_view` and
`view_type`: verify the generated IR contains `pto.make_tensor_view` with shape
`[1]` and strides `[1]`, and that `pto.cmo.cacheinvalid` includes the typed
`!pto.tensor_view<1x...>` result. Preserve the existing `single_cache_line` and
`partition_tensor_view` assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/backend/common/pto_ops_memory.cpp`:
- Around line 861-865: Add regression-test assertions for the scalar view
emitted around `scalar_view` and `view_type`: verify the generated IR contains
`pto.make_tensor_view` with shape `[1]` and strides `[1]`, and that
`pto.cmo.cacheinvalid` includes the typed `!pto.tensor_view<1x...>` result.
Preserve the existing `single_cache_line` and `partition_tensor_view`
assertions.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b9476c8-ff48-4457-964c-723581990f6d

📥 Commits

Reviewing files that changed from the base of the PR and between efb7837 and af8c5b1.

📒 Files selected for processing (1)

src/backend/common/pto_ops_memory.cpp

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af8c5b1a75

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-24T09:56:47Z

+      const std::string scalar_view = codegen.NewTemp();
+      const std::string view_type = "!pto.tensor_view<1x" + dtype_str + ">";
+      codegen.Emit(scalar_view + " = pto.make_tensor_view " + write_ptr +
+                   ", shape = [1], strides = [1] {layout = #pto.layout<nd>} : " + view_type);


Emit SSA index constants for scalar tensor view

When the scalar system.cacheinvalid path is taken, this emits pto.make_tensor_view with bare 1 literals in the shape and strides operand lists. The rest of PTO codegen materializes even static tensor-view dimensions as index SSA values (for example %c1_index) before printing make_tensor_view, so this scalar path will generate MLIR that PTOAS cannot parse for all-ones cacheinvalid regions; please emit/reuse an index constant for 1 and use that SSA name in both lists.

Useful? React with 👍 / 👎.

Fixed in d81e09d: replaced raw 1 literals with codegen.GetOrEmitConstant(static_cast<int64_t>(1), DataType::INDEX) producing %c1_index, matching the pattern used in EmitMakeTensorViews and EmitCommRemoteView. All 7722 UTs pass.

georgebisbas

Addressed in d81e09d: test_cacheinvalid_scalar_write_emits_scalar_view (renamed from _ptr) now asserts pto.make_tensor_view presence and : !pto.tensor_view<1x in the cmo line, which would have caught both the missing view wrapping and the raw integer literal regressions.

…id emit Three polish items from PR hw-native-sys#2137 review: 1. Extract EmitScalarTensorViewPTO helper that consolidates the addptr + make_tensor_view(1xT) pattern for 1-element scalar views. 2. DRY the system.cacheinvalid handler: both scalar and partition-view branches now compute (view_ssa, view_type) and converge to a single shared pto.cmo.cacheinvalid emit — future PTOAS syntax changes only need one emit site update. 3. Strengthen test_cacheinvalid_scalar_write_emits_scalar_view to assert SSA index constants (shape = [%c1_index]) in the scalar make_tensor_view, catching raw integer literal regressions.

… 0.50 The scalar-write branch of system.cacheinvalid codegen in pto_ops_memory.cpp had two PTOAS 0.50 incompatibilities: 1. Missing view operand: emitted pto.cmo.cacheinvalid against a raw !pto.ptr<T> without a type annotation. PTOAS 0.50+ requires a typed tensor_view or partition_view operand. 2. Raw integer literals in make_tensor_view: shape = [1], strides = [1] used bare 1 literals instead of SSA index constants (%c1_index). Fix: extract EmitScalarTensorViewPTO helper that consolidates the addptr + make_tensor_view(1xT) pattern. Both scalar and partition-view branches now converge to a single shared pto.cmo.cacheinvalid emit. Test: strengthen test_cacheinvalid_scalar_write_emits_scalar_view to assert pto.make_tensor_view presence, typed view operand, and SSA index constants in shape/strides — catching the exact regressions that the two PTOAS 0.50 incompatibilities introduced. Fixes: hw-native-sys#2136

Copilot

Pull request overview

Fixes PTOAS 0.50 compatibility for scalar cacheinvalid code generation.

Changes:

Wraps scalar pointers in typed one-element tensor views using SSA index constants.
Unifies scalar and partition-view cacheinvalid emission.
Strengthens regression coverage for generated PTO syntax.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`src/backend/common/pto_ops_memory.cpp`	Adds scalar tensor-view emission and typed cache invalidation.
`tests/ut/codegen/test_pto_codegen_ops.py`	Verifies scalar views, type annotations, and SSA constants.

github-project-automation Bot added this to pto project Jul 24, 2026

coderabbitai Bot reviewed Jul 24, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 24, 2026

View reviewed changes

georgebisbas commented Jul 24, 2026

View reviewed changes

georgebisbas force-pushed the fix/cacheinvalid-ptoas-view branch 2 times, most recently from 69f5146 to 37e3448 Compare July 24, 2026 11:45

georgebisbas mentioned this pull request Jul 24, 2026

chore(toolchain): Update PTOAS to v0.52 and adapt ptoas discovery #2135

Open

3 tasks

coderabbitai Bot mentioned this pull request Jul 24, 2026

revert: InsertCommFence pass — buggy ptoas 0.50 dependency (#2076) #2138

Merged

2 tasks

georgebisbas force-pushed the fix/cacheinvalid-ptoas-view branch from 37e3448 to d4dcf71 Compare July 27, 2026 07:06

Copilot AI review requested due to automatic review settings July 27, 2026 07:06

Copilot started reviewing on behalf of georgebisbas July 27, 2026 07:08 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

luohuan19 mentioned this pull request Jul 28, 2026

Reland InsertCommFence on ptoas 0.52 and unify the cacheinvalid codegen path #2168

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(codegen): wrap scalar cacheinvalid in make_tensor_view for ptoas 0.50 - #2137

fix(codegen): wrap scalar cacheinvalid in make_tensor_view for ptoas 0.50#2137
georgebisbas wants to merge 1 commit into
hw-native-sys:mainfrom
georgebisbas:fix/cacheinvalid-ptoas-view

georgebisbas commented Jul 24, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 24, 2026

Uh oh!

georgebisbas Jul 24, 2026

Uh oh!

georgebisbas left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

georgebisbas commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

georgebisbas Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

georgebisbas left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

georgebisbas commented Jul 24, 2026 •

edited

Loading

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading