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

dialect: Deprecating .get in llvm dialect #1349

Merged
merged 1 commit into from Jul 27, 2023

Conversation

ShaolunWang
Copy link
Collaborator

Making respective changes from .get to init in mpi lowering and llvm/mpi tests

@ShaolunWang ShaolunWang added dialects Changes on the dialects API Related to changes regarding API of constructs minor For minor PRs, easy and quick to review, quickly mergeable labels Jul 26, 2023
@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Patch coverage: 87.50% and no project coverage change.

Comparison is base (d8f4b59) 89.65% compared to head (640af51) 89.65%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1349   +/-   ##
=======================================
  Coverage   89.65%   89.65%           
=======================================
  Files         191      191           
  Lines       24939    25007   +68     
  Branches     3760     3769    +9     
=======================================
+ Hits        22358    22420   +62     
- Misses       1987     1992    +5     
- Partials      594      595    +1     
Files Changed Coverage Δ
xdsl/transforms/lower_mpi.py 92.39% <ø> (ø)
xdsl/dialects/llvm.py 87.62% <85.18%> (-0.93%) ⬇️
tests/dialects/test_llvm.py 100.00% <100.00%> (ø)
xdsl/transforms/printf_to_llvm.py 82.60% <100.00%> (ø)

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@AntonLydike AntonLydike left a comment

Choose a reason for hiding this comment

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

I think this is much easier to review once you make get just call __init__... Otherwise it looks good! Thanks!

xdsl/dialects/llvm.py Outdated Show resolved Hide resolved
xdsl/dialects/llvm.py Outdated Show resolved Hide resolved
Making respective changes from .get to __init__ in
mpi lowering and tests
@ShaolunWang
Copy link
Collaborator Author

Rebased this so github can actually show my changes here 🙃

Copy link
Collaborator

@math-fehr math-fehr left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator

@AntonLydike AntonLydike left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks!

@AntonLydike AntonLydike merged commit 7f5d954 into xdslproject:main Jul 27, 2023
10 checks passed
@ShaolunWang ShaolunWang deleted the llvm-init branch November 19, 2023 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Related to changes regarding API of constructs dialects Changes on the dialects minor For minor PRs, easy and quick to review, quickly mergeable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants