Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retire downstream constant packing folder #921

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

adam-smnk
Copy link
Collaborator

@adam-smnk adam-smnk commented Jun 5, 2024

Replaces our downstream implementation of constant packing folder with upstream patterns. The existing pass selects pack ops to fold and otherwise acts as a small wrapper for the upstream logic.

Also, there was a bug in the element index calculation. After the replacement, folded constants match the outputs produced by runtime packing.

Replaces our downstream implementation of constant packing folder
with upstream patterns. The existing pass acts as a small wrapper
for the upstream logic.

Also, there was a bug in the element index calculation.
After the replacement, the folded constant match the outputs
produced by packing at runtime.
@adam-smnk adam-smnk added the benchmark Triggers benchmark jobs label Jun 5, 2024
@adam-smnk adam-smnk marked this pull request as draft June 5, 2024 11:54
@adam-smnk adam-smnk added benchmark Triggers benchmark jobs and removed benchmark Triggers benchmark jobs labels Jun 5, 2024
@adam-smnk adam-smnk added benchmark Triggers benchmark jobs and removed benchmark Triggers benchmark jobs labels Jun 6, 2024
@adam-smnk adam-smnk marked this pull request as ready for review June 6, 2024 16:43
@adam-smnk
Copy link
Collaborator Author

Rewrite problems fixed. In the initial version, chains of packs e.g., pack_vnni(pack(constant)) would not get folded after a single ConstantFoldPack pass execution. This is because now the rewrite is more iterative: select constant packs -> lower to linalg -> let other patterns constant fold and cleanup the ops.

The lowering to linalg is now a pattern such that greedy rewriter can iterate over the graph and step by step fold all the packs.
The core of the folder logic now relies fully on upstream passes. The only extra bit is operation selection (which packs to fold). However, that is opinionated and very user-specific so, it can probably remain downstream.

Added extra test case to cover this case. No performance regression in benchmarks.

@adam-smnk adam-smnk requested a review from rengolin June 6, 2024 16:49
@adam-smnk adam-smnk merged commit f307b5a into plaidml:main Jun 7, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Triggers benchmark jobs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants