-
Notifications
You must be signed in to change notification settings - Fork 16
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
Implemented GPU OpenCL runtime #343
Conversation
7b81633
to
48e6f1d
Compare
7630169
to
048f66d
Compare
eba9a73
to
2a2e058
Compare
d8daf57
to
8d10a7a
Compare
0ebb9d7
to
b93b5d9
Compare
44af344
to
37d306f
Compare
48dcdc2
to
4efc410
Compare
3ed8730
to
66d8694
Compare
|
||
constexpr char matmulAddStatic[] = R"mlir( | ||
module @fragment_name attributes {"#dlti.sys_spec" = #dlti.target_system_spec<"CPU" : #dlti.target_device_spec<#dlti.dl_entry<"tile_size", 32 : i32>>>} { | ||
func.func @entry(%arg0: memref<64x128xf32>, %arg1: memref<128x128xf32>, %arg2: memref<64x128xf32>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lowering to xegpu.dpas
is not supported with f32
, let's change this test to f16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
I think I'm more or less good with the changes (OV integration works with this runtime). The only thing that keeps me from merging this PR is that we have to temporarily disable XeGPU tests in GC until the gpu-runner is merged. @AndreyPavlenko is there a way of how we can merge this PR and keep both Also, @kurapov-peter, what do you think on merging this one without a tedious review? I think it's the last puzzle piece that keeps us from claiming that OV integration works on GC main branch (technically we also need this one, but we already have an approve there) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm generally OK with the change. I wonder if we really need another logger along with the llvm's one. I'd also prefer to not drop tests. Can we merge the two changes back to back?
The runner is already implemented and the tests pass with it. There are not many changes - 9adeab8 |
To be honest, I don't like the llvm's logger. This one is easier to use: gcLogD("This is a debug message"); VS LLVM_DEBUG(llvm::dbgs() << "This is a debug message\n"); Also, for debug builds, it prints the file and line number, that makes it convenient for in-IDE navigation - single click on a log message navigates to the corresponding line.
But, if required, I could integrate this logger with llvm's one. It's quite simple. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to remove imex from the default GPU passes target.
c055fad
to
b6ed40b
Compare
Added the |
How to use:
See the unit test.
Depends on #333 and #329