-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[MLIR][Math][XeVM] Reintroduce MathToXeVM (math-to-xevm) pass #162934
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
Conversation
|
Sidenote: Is there a way to trigger the postcommit runner tests before merging? I was able to replicate the issue and fix it on my local machine, but nonetheless confirmation would be helpful. |
|
I can see all the changes in this PR. I think you need to either revert the previous PR and apply this or only add the missing link libraries. |
A revert was merged at #162923, I think I have to land my prior PR again |
ah great. thanks! Tip: you can avoid linking issues by doing a local shared_lib build. |
Thank you for the tip, I unfortunately had to learn this the hard way 😅 It builds now with Are there anything else that I should watch out for? |
AFAIK That's all you can do + PR CI. sometimes it can still fail in post merge CI. for those the only solution is to revert unfortunately. |
|
Checked everything over one last time, PR should be good now to merge |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/31408 Here is the relevant piece of the build log for the reference |
AFAICT this seems to be a CI issue; the same command runs fine on my machine. Please let me know if there are problems however. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/11940 Here is the relevant piece of the build log for the reference |
|
Sanitizer failures can also be observed in other PRs: #163302 Neither my commit nor the mentioned commit has anything to do with clang, failure should be unrelated. |
…62934) This PR is a fix for llvm#159878, which failed in postcommit testing due to linker errors that were not caught in precommit. Original PR: --- This PR introduces a `MathToXeVM` pass, which implements support for the `afn` fastmath flag for SPIRV/XeVM targets - It takes supported `Math` Ops with the `afn` flag, and converts them to function calls to OpenCL `native_` intrinsics. These intrinsic functions are supported by the SPIRV backend, and are automatically converted to `OpExtInst` calls to `native_` ops from the OpenCL SPIRV ext. inst. set when outputting to SPIRV/XeVM. Note: - This pass also supports converting `arith.divf` to native equivalents. There is an option provided in the pass to turn this behavior off. - This pass preserves fastmath flags, but these flags are currently ignored by the SPIRV backend. Thus, in order to generate SPIRV that truly preserves fastmath flags, support needs to be added to the SPIRV backend.
This passes buildkite CI, but fails downstream if the SPIRV target is not enabled. This is needed after #162934
| @@ -0,0 +1,118 @@ | |||
| // RUN: mlir-opt %s -gpu-module-to-binary="format=isa" \ | |||
| // RUN: -debug-only=serialize-to-isa 2> %t | |||
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 pushed eca6144 to fix this test in release builds.
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.
Thanks for the fix!
This PR is a fix for #159878, which failed in postcommit testing due to linker errors that were not caught in precommit.
Original PR:
This PR introduces a
MathToXeVMpass, which implements support for theafnfastmath flag for SPIRV/XeVM targets - It takes supportedMathOps with theafnflag, and converts them to function calls to OpenCLnative_intrinsics.These intrinsic functions are supported by the SPIRV backend, and are automatically converted to
OpExtInstcalls tonative_ops from the OpenCL SPIRV ext. inst. set when outputting to SPIRV/XeVM.Note:
arith.divfto native equivalents. There is an option provided in the pass to turn this behavior off.