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

API: Implement and use the SymbolTable Trait #1338

Merged
merged 16 commits into from Jul 27, 2023
Merged

Conversation

PapyChacal
Copy link
Collaborator

@PapyChacal PapyChacal commented Jul 24, 2023

The SymbolTable trait offer Symbol lookup utilites and ensure Symbol uniqueness amongst its children Symbols.
This implements it, tests it, and use it on implemented operations, that is: builtin.module, gpu.module, and irdl.dialect.
For reference: https://mlir.llvm.org/docs/SymbolsAndSymbolTables/
NB: This documentation states "Described above are Symbols, which reside within a region of an operation defining a SymbolTable." but I could not find any enforcment of this anywhere in the codebase, so I didn't implement it. If anyone finds it, please tell 🙂

@PapyChacal PapyChacal added the API Related to changes regarding API of constructs label Jul 24, 2023
@PapyChacal PapyChacal self-assigned this Jul 24, 2023
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Patch coverage: 93.90% and project coverage change: +0.01% 🎉

Comparison is base (080856a) 89.66% compared to head (5499dab) 89.68%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1338      +/-   ##
==========================================
+ Coverage   89.66%   89.68%   +0.01%     
==========================================
  Files         193      193              
  Lines       25043    25117      +74     
  Branches     3770     3786      +16     
==========================================
+ Hits        22456    22526      +70     
- Misses       1992     1994       +2     
- Partials      595      597       +2     
Files Changed Coverage Δ
xdsl/dialects/gpu.py 97.36% <ø> (ø)
xdsl/traits.py 92.08% <86.11%> (-1.19%) ⬇️
tests/test_traits.py 99.47% <100.00%> (+0.13%) ⬆️
xdsl/dialects/builtin.py 84.75% <100.00%> (+0.02%) ⬆️
xdsl/dialects/irdl/irdl.py 94.97% <100.00%> (ø)
xdsl/ir/core.py 87.82% <100.00%> (ø)

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

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.

LGTM modulo minor changes, thank you for doing this

tests/test_traits.py Outdated Show resolved Hide resolved
xdsl/dialects/builtin.py Outdated Show resolved Hide resolved
xdsl/traits.py Outdated Show resolved Hide resolved
xdsl/traits.py Outdated Show resolved Hide resolved
xdsl/traits.py Outdated Show resolved Hide resolved
PapyChacal and others added 12 commits July 25, 2023 12:22
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
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.

Very nice!

tests/test_traits.py Outdated Show resolved Hide resolved
xdsl/traits.py Outdated Show resolved Hide resolved
@PapyChacal PapyChacal merged commit 1a467bc into main Jul 27, 2023
10 checks passed
@PapyChacal PapyChacal deleted the emilien/symbol-table branch July 27, 2023 09:15
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants