Skip to content

[SYCL][SPIRV] Implement islessgreater with FOrdNotEqual instead #5076

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

Merged
merged 5 commits into from
Dec 9, 2021

Conversation

Nuullll
Copy link
Contributor

@Nuullll Nuullll commented Dec 3, 2021

SPIR-V OpLessOrGreater is deprecated:
https://www.khronos.org/registry/SPIR-V/specs/unified1/SPIRV.html#OpLessOrGreater

OpFOrdNotEqual has the same semantics as OpLessOrGreater.

Signed-off-by: Yilong Guo yilong.guo@intel.com

SPIR-V OpLessOrGreater is deprecated:
https://www.khronos.org/registry/SPIR-V/specs/unified1/SPIRV.html#OpLessOrGreater

OpFOrdNotEqual has the same semantics as OpLessOrGreater.
@Nuullll
Copy link
Contributor Author

Nuullll commented Dec 3, 2021

First-time contributors need a maintainer to approve running workflows.

Could @bader please approve running workflows? Many thanks!

Signed-off-by: Yilong Guo <yilong.guo@intel.com>
Signed-off-by: Yilong Guo <yilong.guo@intel.com>
// (LessOrGreater) // islessgreater
__SYCL_EXPORT s::cl_int LessOrGreater(s::cl_float x, s::cl_float y) __NOEXC {
return __sLessOrGreater(x, y);
// (FOrdNotEqual) // islessgreater
Copy link
Contributor

Choose a reason for hiding this comment

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

@Nuullll, please, follow Contribution guide and pay attention to ABI policies in particular.
I think we can't remove LessOrGreater immediately and should rather deprecate them first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @bader. That makes sense since this is an ABI-breaking change.
Then may I ask how to deprecate an ABI?

Copy link
Contributor

Choose a reason for hiding this comment

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

how to deprecate an ABI?

@romanovvlad, @s-kanaev, please, add an answer to this question to ABI Policy Guide.

@bader bader requested review from a team and dm-vodopyanov and removed request for a team December 3, 2021 07:52
@Nuullll Nuullll changed the title [WIP][SYCL][SPIRV] Replace deprecated LessOrGreater with FOrdNotEqual [WIP][SYCL][SPIRV][ABI Breaking] Replace deprecated LessOrGreater with FOrdNotEqual Dec 6, 2021
@Nuullll Nuullll changed the title [WIP][SYCL][SPIRV][ABI Breaking] Replace deprecated LessOrGreater with FOrdNotEqual [WIP][SYCL][SPIRV] Implement islessgreater with FOrdNotEqual instead Dec 7, 2021
@Nuullll Nuullll requested a review from bader December 7, 2021 05:41
@bader
Copy link
Contributor

bader commented Dec 7, 2021

@Nuullll, are you still working on it? Or just just forgot to push "Ready for review" button?

@Nuullll Nuullll marked this pull request as ready for review December 7, 2021 07:37
@Nuullll Nuullll requested a review from a team as a code owner December 7, 2021 07:37
@Nuullll Nuullll changed the title [WIP][SYCL][SPIRV] Implement islessgreater with FOrdNotEqual instead [SYCL][SPIRV] Implement islessgreater with FOrdNotEqual instead Dec 7, 2021
@Nuullll
Copy link
Contributor Author

Nuullll commented Dec 7, 2021

@Nuullll, are you still working on it? Or just just forgot to push "Ready for review" button?

Thanks for reminding. Please help review it :-P

Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

LGTM, except deprecating old API.
@intel/llvm-reviewers-runtime, ping.

@bader
Copy link
Contributor

bader commented Dec 9, 2021

@intel/llvm-reviewers-runtime, ping.

ping #2.

@dm-vodopyanov dm-vodopyanov merged commit 0b8df3b into intel:sycl Dec 9, 2021
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Dec 11, 2021
* upstream/sycl: (725 commits)
  [SYCL] Translate ZE_RESULT_ERROR_INVALID_ARGUMENT error code from L0 RT (intel#5122)
  [SYCL][L0][Plugin] Call ZeCommandQueueCreate on demand (intel#5109)
  [SYCL] Switch to using blocking USM free for OpenCL GPU (intel#4928)
  [CI] Disable pack and upload steps (intel#5119)
  [SYCL] Disable submission of AssertInfoCopier for FPGA (intel#4780)
  [SYCL][SPIRV] Implement islessgreater with FOrdNotEqual instead (intel#5076)
  [SYCL] Fix typo in the name of the host-visible pool (intel#5073)
  [SYCL] Only call shutdown when DLL is being unloaded, not when process is terminating (intel#4983)
  [SYCL][CUDA][PI] Fix infinite loop when parallel_for range exceeds INT_MAX (intel#5095)
  [SYCL] Translate out-of-memory error codes from L0 RT (intel#5107)
  [SYCL] Fix a few warnings during build scripts configuration (intel#5082)
  [SYCL] Fix amdgpu openmp test (intel#5103)
  [SYCL] [FPGA] Create experimental headers for FPGA latency control (intel#5066)
  [SYCL][CUDA] Don't enqueue an event wait on same CUDA stream (intel#5099)
  Remove PR disable template (intel#5102)
  [BuildBot]Uplift CPU/FPGAEMU RT version (intel#5078)
  [SYCL] Fix the test to not depend on a specific line. (intel#5092)
  [CI] Provide libclc targets to build and test (intel#5091)
  Fix build of `check-llvm-spirv` target after 8f8001a
  Force opt to use new pass manager in pr52289 test after c34d157
  ...
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Dec 12, 2021
* upstream/sycl:
  [CI] Add container users to video group (intel#5101)
  [CI] More typo fixes in Nightly build (intel#5088)
  Revert "[CI] Disable pack and upload steps (intel#5119)" (intel#5125)
  [SYCL] Translate ZE_RESULT_ERROR_INVALID_ARGUMENT error code from L0 RT (intel#5122)
  [SYCL][L0][Plugin] Call ZeCommandQueueCreate on demand (intel#5109)
  [SYCL] Switch to using blocking USM free for OpenCL GPU (intel#4928)
  [CI] Disable pack and upload steps (intel#5119)
  [SYCL] Disable submission of AssertInfoCopier for FPGA (intel#4780)
  [SYCL][SPIRV] Implement islessgreater with FOrdNotEqual instead (intel#5076)
  [SYCL] Fix typo in the name of the host-visible pool (intel#5073)
  [SYCL] Only call shutdown when DLL is being unloaded, not when process is terminating (intel#4983)
  [SYCL][CUDA][PI] Fix infinite loop when parallel_for range exceeds INT_MAX (intel#5095)
  [SYCL] Translate out-of-memory error codes from L0 RT (intel#5107)
  [SYCL] Fix a few warnings during build scripts configuration (intel#5082)
  [SYCL] Fix amdgpu openmp test (intel#5103)
  [SYCL] [FPGA] Create experimental headers for FPGA latency control (intel#5066)
  [SYCL][CUDA] Don't enqueue an event wait on same CUDA stream (intel#5099)
  Remove PR disable template (intel#5102)
  [BuildBot]Uplift CPU/FPGAEMU RT version (intel#5078)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants