Skip to content

Comments

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

Merged
dm-vodopyanov merged 5 commits intointel:syclfrom
Nuullll:LessOrGreater
Dec 9, 2021
Merged

[SYCL][SPIRV] Implement islessgreater with FOrdNotEqual instead#5076
dm-vodopyanov merged 5 commits intointel:syclfrom
Nuullll:LessOrGreater

Conversation

@Nuullll
Copy link
Contributor

@Nuullll Nuullll commented Dec 3, 2021 •
edited
Loading

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

@Nuullll
[SYCL][SPIRV] Replace deprecated LessOrGreater with FOrdNotEqual
3e1f311 
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!

@Nuullll
Fix ABI symbols
a3cb6b9 
Signed-off-by: Yilong Guo <yilong.guo@intel.com>
@Nuullll
Add device code test
113a491 
Signed-off-by: Yilong Guo <yilong.guo@intel.com>
bader
bader requested changes Dec 3, 2021
View reviewed changes
sycl/source/detail/builtins_relational.cpp
// (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

@bader bader Dec 3, 2021

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

@Nuullll Nuullll Dec 6, 2021

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

@bader bader Dec 6, 2021

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 •
edited
Loading

@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

bader
bader approved these changes Dec 7, 2021
View reviewed changes
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
@alexbatashev
Merge remote-tracking branch 'upstream/sycl' into hip_cuda_builds
f7efebe 
* 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
@alexbatashev
Merge remote-tracking branch 'upstream/sycl' into llvm_test_suite
ec072b1 
* 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

Reviewers

@bader bader bader approved these changes

@dm-vodopyanov dm-vodopyanov dm-vodopyanov approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Nuullll @bader @dm-vodopyanov