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

Arith to ModArith Conversion pass and Mac Transformer pass #1125

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

WoutLegiest
Copy link
Collaborator

@WoutLegiest WoutLegiest commented Nov 27, 2024

  • Adding conversion pass to convert arith into mod arith; now limited operations are supported.
    Conversion is place in the Arith folder, but lives in the heir::arith dialect
  • Transformation pass in mod-arith to find consecutive mul and add operations and replace them with a Mac operation
  • Pipeline pass for Tosa to secret/Arith i32

The arith-to-mod-arith pass is required to lower a neural network TOSA model to a CGGI backend. This pass will transform the operations to the mod-arith dialect, where the find-mac pass can be used to convert consecutive multiply addition operations into a single operation. In a later pass (ongoing work), these large precision MAC operations (typically 64 or 32-bit) will be lowered into small precision (8 or 4b) operations that can be mapped to CGGI operations.

@j2kun
Copy link
Collaborator

j2kun commented Dec 3, 2024

Looks like some tests are failing. Could you fix that and then send the PR to me for review?

@WoutLegiest WoutLegiest force-pushed the macpass branch 4 times, most recently from 5135ddf to 40e5db2 Compare December 5, 2024 19:25
@j2kun
Copy link
Collaborator

j2kun commented Dec 5, 2024

One thing I think would be helpful is an explanation (perhaps in the PR description, maybe also in the tablegen pass description field) of the broader reason for having this pass. What e2e use case is it intended to support, and what stage of that e2e use case is it managing?

@WoutLegiest WoutLegiest requested a review from j2kun December 5, 2024 23:29
@WoutLegiest WoutLegiest force-pushed the macpass branch 2 times, most recently from 8221333 to baca68e Compare December 5, 2024 23:43
@j2kun
Copy link
Collaborator

j2kun commented Dec 10, 2024

@WoutLegiest is this ready for a final review?

@WoutLegiest
Copy link
Collaborator Author

@WoutLegiest is this ready for a final review?

Yes it is!

@WoutLegiest WoutLegiest requested a review from j2kun December 10, 2024 23:44
Copy link
Collaborator

@AlexanderViand-Intel AlexanderViand-Intel left a comment

Choose a reason for hiding this comment

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

I don't think I fully understand why this TOSA pipeline needs to use mod_arith but I already have some ideas for howto (ab)use ArithToModArith so super happy to see it :)

lib/Dialect/ModArith/Transforms/ConvertToMac.cpp Outdated Show resolved Hide resolved
lib/Dialect/ModArith/Transforms/Passes.td Outdated Show resolved Hide resolved
lib/Pipelines/BooleanPipelineRegistration.cpp Show resolved Hide resolved
lib/Utils/ConversionUtils/ConversionUtils.cpp Show resolved Hide resolved
@j2kun j2kun added the pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing label Dec 12, 2024
@copybara-service copybara-service bot merged commit 779a90a into google:main Dec 12, 2024
10 of 11 checks passed
@WoutLegiest WoutLegiest deleted the macpass branch December 12, 2024 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants