Skip to content

[Mlir] Support buffer_at with SDist#62

Open
ElectrikSpace wants to merge 1 commit intoxtc-tools:mainfrom
ElectrikSpace:dev/snoiry/mlir_buffer_at
Open

[Mlir] Support buffer_at with SDist#62
ElectrikSpace wants to merge 1 commit intoxtc-tools:mainfrom
ElectrikSpace:dev/snoiry/mlir_buffer_at

Conversation

@ElectrikSpace
Copy link
Contributor

Motivation

Propose an implementation of buffer_at in the Mlir backend.

Description

Support the buffer_at scheduling primitive using SDist

Commits

Single commit

Discussion

Like pack_at, the implementation does nothing if SDist is not installed

@ElectrikSpace ElectrikSpace requested a review from qaco March 6, 2026 16:49
Comment on lines +561 to +562
graph_backend = cast(MlirGraphBackend, self._mlir_schedule.scheduler.backend)
node_backend = cast(MlirNodeBackend, graph_backend.nodes[schedule.node_name])
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to pass the pyright type checks

Copy link
Contributor

@qaco qaco Mar 6, 2026

Choose a reason for hiding this comment

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

Could you use assert isinstance(var,type) instead ? Imho cast is ambiguous (in terms of intent)

memref.ApplyFoldMemrefAliasOpsPatternsOp()
if "sdist" in self._mlir_program.mlir_extensions:
assert sdist_transform is not None
sdist_transform.SDistLocalBufferAtOp(
Copy link
Contributor

Choose a reason for hiding this comment

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

What does it do under the hood ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It uses the same SDist primitive as pack_at, which automatically creates a local buffer at a particular loop level. The transformation automatically infer which buffer is a read and/or write.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok could you test the pipeline at different levels, aka transform, then transformed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not, but SDist is not tested in CI. Maybe We should ask @guillon how to test XTC also with SDist

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes if we use it for all targets we should test it

@qaco
Copy link
Contributor

qaco commented Mar 6, 2026

Could you provide small, self-contained filecheck tests for this feature ? For instance in tests/filecheck/mlir_loop

@ElectrikSpace
Copy link
Contributor Author

I can create tests equivalents to tests/filecheck/backends/test_mlir_pack_sdist.py and tests/filecheck/backends/test_mlir_pack_no_sdist.py

@qaco
Copy link
Contributor

qaco commented Mar 6, 2026

I can create tests equivalents to tests/filecheck/backends/test_mlir_pack_sdist.py and tests/filecheck/backends/test_mlir_pack_no_sdist.py

ok!

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.

2 participants