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.

Sign up for GitHub

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

Jump to bottom

[MLIR][XeGPU] Add dpas and named barrier ops #88973

Merged
merged 22 commits into from
Apr 24, 2024
Merged

[MLIR][XeGPU] Add dpas and named barrier ops #88973

merged 22 commits into from
Apr 24, 2024

Conversation

chencha3
Copy link
Contributor

Adds definition of dpas op, atomic op, named barrier and related ops.

chencha3 and others added 11 commits March 25, 2024 22:30
@chencha3
Add XeGPU scattered ops
0f86b93
@chencha3 @joker-eph
Update mlir/lib/Dialect/XeGPU/IR/XeGPUOps.cpp
89148e9 
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
@chencha3 @adam-smnk
Update mlir/include/mlir/Dialect/XeGPU/IR/XeGPUOps.td
2c3bd13 
Co-authored-by: Adam Siemieniuk <adam.siemieniuk@intel.com>
@chencha3
refine getShapeOf implementation
6486c99
@chencha3 @adam-smnk
Update mlir/include/mlir/Dialect/XeGPU/IR/XeGPUOps.td
ff28836 
Co-authored-by: Adam Siemieniuk <adam.siemieniuk@intel.com>
@chencha3 @joker-eph
Update mlir/lib/Dialect/XeGPU/IR/XeGPUOps.cpp
a375116 
Co-authored-by: Mehdi Amini <joker.eph@gmail.com>
@chencha3
improve doc
4ca38ed
@chencha3
Merge branch 'users/chencha3/xegpu_scatter_ops_upstream' of https://g…
8c305fe 
…ithub.com/llvm/llvm-project into xegpu_scatter_ops_upstream
@chencha3
add invalid test cases
253b96f
@chencha3
add an overlapping example for createDesc.
ba62715
@chencha3
Merge branch 'main' into xegpu_scatter_ops_upstream
eedffe6
@github-actions GitHub Actions
Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@chencha3 chencha3 mentioned this pull request Apr 16, 2024
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 16, 2024 •
edited
Loading

✅ With the latest revision this PR passed the C/C++ code formatter.

@chencha3 chencha3 requested a review from joker-eph April 16, 2024 19:51
@nikic nikic removed request for nikic and a team April 18, 2024 09:21
@chencha3
Copy link
Contributor Author

@joker-eph Hi Mehdi, could you please take a look?

@chencha3
Copy link
Contributor Author

Hi @adam-smnk, could you help to have a look and approve also if it looks good to you?

adam-smnk
adam-smnk approved these changes Apr 24, 2024
View reviewed changes
Copy link
Contributor

@adam-smnk adam-smnk left a comment

Choose a reason for hiding this comment

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

Looks good, thanks 👍

@chencha3 chencha3 merged commit 03bb10d into llvm:main Apr 24, 2024
4 checks passed
@joker-eph
Copy link
Collaborator

Bot is broken:

ld.lld: error: undefined symbol: mlir::arith::AtomicRMWKindAttr::get(mlir::MLIRContext*, mlir::arith::AtomicRMWKind)
>>> referenced by XeGPUOps.cpp
>>>               tools/mlir/lib/Dialect/XeGPU/IR/CMakeFiles/obj.MLIRXeGPUDialect.dir/XeGPUOps.cpp.o:(mlir::xegpu::AtomicRMWOp::setKind(mlir::arith::AtomicRMWKind))
>>> referenced by XeGPUOps.cpp

Can you quickly fix? Otherwise we should just revert in the meantime.

@chencha3
Copy link
Contributor Author

Bot is broken:

ld.lld: error: undefined symbol: mlir::arith::AtomicRMWKindAttr::get(mlir::MLIRContext*, mlir::arith::AtomicRMWKind)
>>> referenced by XeGPUOps.cpp
>>>               tools/mlir/lib/Dialect/XeGPU/IR/CMakeFiles/obj.MLIRXeGPUDialect.dir/XeGPUOps.cpp.o:(mlir::xegpu::AtomicRMWOp::setKind(mlir::arith::AtomicRMWKind))
>>> referenced by XeGPUOps.cpp

Can you quickly fix? Otherwise we should just revert in the meantime.

Yes, I am going to fix it now.

@klausler
Copy link
Contributor

The https://lab.llvm.org/buildbot/#/builders/160 build bot (and two others) broke with this change. Please revert or fix immediately.

@chencha3
Copy link
Contributor Author

@joker-eph @klausler I fixed it. How should I proceed? I created a PR for it just in case: #89991

@klausler
Copy link
Contributor

If you have a fix that works, then merge it. Otherwise, revert the original commit that broke the build bots.

@chencha3
Copy link
Contributor Author

chencha3 commented Apr 24, 2024 •
edited
Loading

If you have a fix that works, then merge it. Otherwise, revert the original commit that broke the build bots.

Sorry, I didn't get it how it works? you mean merge to the main branch directly and push it without PR?

@chencha3
Copy link
Contributor Author

If you have a fix that works, then merge it. Otherwise, revert the original commit that broke the build bots.

It seems I don't have permission to push directly. I got this error:
unable to access 'https://github.com/llvm/llvm-project.git/': The requested URL returned error: 403

@klausler
Copy link
Contributor

How did you merge the change that broke the build bots?

@chencha3
Copy link
Contributor Author

I was through the WebUI. So maybe it is better to approve this #89991, and then merge?

@klausler
Copy link
Contributor

If you can get a fast approval, sure. Otherwise, you can merge it without approval, or just revert the original commit.

chencha3 added a commit that referenced this pull request Apr 24, 2024
@chencha3
[MLIR][XeGPU] Fix Bot failure for #88973 (#89991)
186952f
@chencha3
Copy link
Contributor Author

If you can get a fast approval, sure. Otherwise, you can merge it without approval, or just revert the original commit.

merged it, I am going to monitoring the Bot to make sure it passed.

@joker-eph
Copy link
Collaborator

Just merging a fix works as well: the priority is always to get the bot back green, so either the fix is trivial and you just merge it ASAP or we revert and then land a new PR with the fix.

If you have a fix that works, then merge it. Otherwise, revert the original commit that broke the build bots.

It seems I don't have permission to push directly. I got this error: unable to access 'https://github.com/llvm/llvm-project.git/': The requested URL returned error: 403

This is because you didn't setup your repo correctly locally, you need to setup an SSH key and use the SSH URL for the remote here.

@chencha3
Copy link
Contributor Author

Yes, the fix is trivial, and it is merged now. the bot is back green. I am wondering whether there is a guideline about tests need to be done before the PR to avoid such situation as much as possible.

@joker-eph
Copy link
Collaborator

It's hard to catch everything: in general when merging you need to be on the lookout for bot emails.

In this case it's about building with BUILD_SHARED_LIBS=ON

@MaskRay
Copy link
Member

MaskRay commented Apr 24, 2024

For CMake changes, it's recommend to test test both default and -DBUILD_SHARED_LIBS=on. The latter adds -Wl,-z,defs and checks some dependency issue.

@chencha3 chencha3 deleted the xegpu_other_ops_upstream branch July 25, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adam-smnk adam-smnk adam-smnk approved these changes

@joker-eph joker-eph joker-eph approved these changes

@daniel-grumberg daniel-grumberg Awaiting requested review from daniel-grumberg

@cyndyishida cyndyishida Awaiting requested review from cyndyishida

@aaupov aaupov Awaiting requested review from aaupov

@maksfb maksfb Awaiting requested review from maksfb

@rafaelauler rafaelauler Awaiting requested review from rafaelauler

@ayermolo ayermolo Awaiting requested review from ayermolo

@dcci dcci Awaiting requested review from dcci

@JDevlieghere JDevlieghere Awaiting requested review from JDevlieghere

@tbaederr tbaederr Awaiting requested review from tbaederr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@chencha3 @joker-eph @klausler @MaskRay @adam-smnk