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

dialects: add linalg.fill #2189

Merged
merged 13 commits into from
Feb 19, 2024
Merged

dialects: add linalg.fill #2189

merged 13 commits into from
Feb 19, 2024

Conversation

kayode-gif
Copy link
Collaborator

No description provided.

@kayode-gif kayode-gif added the dialects Changes on the dialects label Feb 19, 2024
@kayode-gif kayode-gif self-assigned this Feb 19, 2024
xdsl/dialects/linalg.py Outdated Show resolved Hide resolved
@superlopuh
Copy link
Member

As you can see from the CI output, the fill operation in MLIR expects a scalar input, not a tensor. We might want to implement the same verification. The best thing to do would be to look at the MLIR source code to find the verifier for the fill op

@kayode-gif kayode-gif changed the title (dialects): add linalg.fill dialects: add linalg.fill Feb 19, 2024
Copy link

codecov bot commented Feb 19, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (1cdfdf2) 89.36% compared to head (118e60e) 89.38%.
Report is 9 commits behind head on main.

Files Patch % Lines
xdsl/dialects/linalg.py 77.77% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2189      +/-   ##
==========================================
+ Coverage   89.36%   89.38%   +0.01%     
==========================================
  Files         316      317       +1     
  Lines       38413    38522     +109     
  Branches     5678     5693      +15     
==========================================
+ Hits        34328    34433     +105     
- Misses       3282     3287       +5     
+ Partials      803      802       -1     

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

@kayode-gif kayode-gif merged commit 9a556a9 into main Feb 19, 2024
10 checks passed
@kayode-gif kayode-gif deleted the oke/add-linalg-fill branch February 19, 2024 18:01
superlopuh pushed a commit that referenced this pull request Feb 22, 2024
@gabrielrodcanal
Copy link
Contributor

Haven't really checked the custom format in the operation definition in MLIR, but I've found an example in which -> type($res) is optional (I'm referring to

"`outs` `(` $outputs `:` type($outputs) `)` `->` type($res) attr-dict"
). See the example here: https://github.com/Xilinx/mlir-air/blob/82d23361cb3843e5d8fc01d746d513f25a224125/test/airhost/51_air_mmult_2x2_channel_broadcast/air.mlir#L30

I'm not sure if this was a mistake or it's just provisional until the cutom format API supports optional groups, but just in case I thought I'd have you know!

@superlopuh
Copy link
Member

Oh interesting, should be easy to fix, can you please file an issue with this info?

@gabrielrodcanal
Copy link
Contributor

See #2228.

@superlopuh superlopuh linked an issue Nov 8, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialects Changes on the dialects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dialects: (linalg) add linalg.fill
3 participants