Skip to content

[mod_arith] move arith_ext dialect to mod_arith#907

Merged
copybara-service[bot] merged 1 commit into
google:mainfrom
inbelic:inbelic/arith-ext-to-mod-arith
Aug 14, 2024
Merged

[mod_arith] move arith_ext dialect to mod_arith#907
copybara-service[bot] merged 1 commit into
google:mainfrom
inbelic:inbelic/arith-ext-to-mod-arith

Conversation

@inbelic

@inbelic inbelic commented Aug 13, 2024

Copy link
Copy Markdown
Contributor
- originally arith_ext was added to house a couple extra
  (modulus-based) operations to help represent the optimizations in
  the HEaan.mlir paper
- however, there has been a need for other modular arithmetic
  operations and they have been added to the arith_ext dialect
- as such, it was decided to move this to a specific mod_arith
  dialect to be more concrete and better describe the operations

@inbelic

inbelic commented Aug 13, 2024

Copy link
Copy Markdown
Contributor Author

I haven't built this locally with CMake. Is that covered in the workflow here?

@inbelic inbelic force-pushed the inbelic/arith-ext-to-mod-arith branch from 5ba2fb4 to 3cc78f4 Compare August 13, 2024 20:41
@j2kun

j2kun commented Aug 14, 2024

Copy link
Copy Markdown
Collaborator

The CMake build isn't in the CI yet, but until it is I think it's best effort. Thanks for giving it a go.

@j2kun

j2kun commented Aug 14, 2024

Copy link
Copy Markdown
Collaborator

If you're not using pre-commit, try applying the patch manually: https://github.com/google/heir/actions/runs/10376987295/job/28774854361?pr=907

@inbelic inbelic force-pushed the inbelic/arith-ext-to-mod-arith branch from 41e7407 to 40ef2fe Compare August 14, 2024 17:57
@inbelic

inbelic commented Aug 14, 2024

Copy link
Copy Markdown
Contributor Author

Whoops, forgot to re-install pre-commit on my new install.
Force-pushed to rebase onto main and resolve conflicts

@inbelic inbelic force-pushed the inbelic/arith-ext-to-mod-arith branch from 40ef2fe to d65f797 Compare August 14, 2024 18:00

@asraa asraa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@asraa asraa added the pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing label Aug 14, 2024
    - originally arith_ext was added to house a couple extra
      (modulus-based) operations to help represent the optimizations in
      the HEaan.mlir paper
    - however, there has been a need for other modular arithmetic
      operations and they have been added to the arith_ext dialect
    - as such, it was decided to move this to a specific mod_arith
      dialect to be more concrete and better describe the operations
@inbelic inbelic force-pushed the inbelic/arith-ext-to-mod-arith branch from d65f797 to 88cc268 Compare August 14, 2024 18:40
@copybara-service copybara-service Bot merged commit e9e8f92 into google:main Aug 14, 2024
@inbelic inbelic deleted the inbelic/arith-ext-to-mod-arith branch August 14, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dialect: mod_arith 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.

4 participants