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

core: Implement and use IsolatedFromAbove #1122

Merged
merged 13 commits into from Jun 12, 2023
Merged

Conversation

PapyChacal
Copy link
Collaborator

Implements IsolatedFromAbove and use it on existing MLIR operations that are supposed to.
MLIR's implementation: https://mlir.llvm.org/doxygen/IR_2Operation_8cpp_source.html#l01231

@PapyChacal
Copy link
Collaborator Author

PapyChacal commented Jun 12, 2023

I wasn't sure about riscv_func.func; so left it out. @superlopuh, Do you know if it should be IsolatedFromAbove?

@codecov
Copy link

codecov bot commented Jun 12, 2023

Codecov Report

Patch coverage: 98.11% and project coverage change: +0.05 🎉

Comparison is base (032af0b) 87.01% compared to head (a4a0649) 87.07%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1122      +/-   ##
==========================================
+ Coverage   87.01%   87.07%   +0.05%     
==========================================
  Files         139      139              
  Lines       20914    20964      +50     
  Branches     3146     3163      +17     
==========================================
+ Hits        18199    18254      +55     
+ Misses       2193     2191       -2     
+ Partials      522      519       -3     
Impacted Files Coverage Δ
xdsl/traits.py 95.55% <92.30%> (+1.61%) ⬆️
tests/test_builtin_traits.py 100.00% <100.00%> (ø)
xdsl/dialects/builtin.py 83.68% <100.00%> (+0.04%) ⬆️
xdsl/dialects/func.py 94.64% <100.00%> (+0.03%) ⬆️
xdsl/dialects/riscv.py 87.13% <100.00%> (+0.14%) ⬆️
xdsl/dialects/stencil.py 95.28% <100.00%> (+0.03%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

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.

Coolio! just one thing on traversing the IR

xdsl/traits.py Show resolved Hide resolved
xdsl/traits.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@compor compor left a comment

Choose a reason for hiding this comment

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

LGTM

xdsl/traits.py Outdated Show resolved Hide resolved
tests/test_builtin_traits.py Show resolved Hide resolved
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.

Really nice!

xdsl/dialects/riscv.py Show resolved Hide resolved
xdsl/traits.py Outdated Show resolved Hide resolved
xdsl/traits.py Outdated Show resolved Hide resolved
Copy link
Member

@superlopuh superlopuh left a comment

Choose a reason for hiding this comment

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

very nice

PapyChacal and others added 2 commits June 12, 2023 11:52
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Copy link
Collaborator

@webmiche webmiche left a comment

Choose a reason for hiding this comment

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

Nice, LGTM!

xdsl/traits.py Outdated Show resolved Hide resolved
@PapyChacal
Copy link
Collaborator Author

@compor Added a nested test. Does it look like what you had in mind? 🙂

@PapyChacal PapyChacal merged commit 3143265 into main Jun 12, 2023
12 checks passed
@PapyChacal PapyChacal deleted the emilien/isolated-from-above branch June 12, 2023 15:17
cigarichard pushed a commit to cigarichard/xdsl that referenced this pull request Jun 17, 2023
Implements IsolatedFromAbove and use it on existing MLIR operations that
are supposed to.
MLIR's implementation:
https://mlir.llvm.org/doxygen/IR_2Operation_8cpp_source.html#l01231

---------

Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core xDSL core (ir, textual format, ...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants