Skip to content

[SYCL] Fix numeric_limits<half> after intel/llvm#1089 #1138

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

Conversation

AlexeySachkov
Copy link
Contributor

Error was reproducible in two cases:

  • using something like numeric_limits<half>::min() in within another
    constexpr
  • not treating SYCL headers as system ones with -Winvalid-constexpr
    treated as error

Error was reproducible in two cases:
- using something like `numeric_limits<half>::min()` in within another
  `constexpr`
- not treating SYCL headers as system ones with `-Winvalid-constexpr`
  treated as error

Signed-off-by: Alexey Sachkov <alexey.sachkov@intel.com>
@AlexeySachkov AlexeySachkov force-pushed the private/asachkov/fix-numeric-limit-half branch from b7db5fb to 08da2e8 Compare February 18, 2020 10:01
@bader
Copy link
Contributor

bader commented Feb 18, 2020

Please, fix lit test failure.

@AlexeySachkov
Copy link
Contributor Author

Please, fix lit test failure.

This failure is not related to this patch - this is known sporadic issue

@bader
Copy link
Contributor

bader commented Feb 20, 2020

Please, fix lit test failure.

This failure is not related to this patch - this is known sporadic issue

I can't merge until test passes. Who is looking into this issue?

@AlexeySachkov AlexeySachkov requested a review from bader February 25, 2020 14:46
@bader bader merged commit 85d7a5e into intel:sycl Feb 25, 2020
@AlexeySachkov AlexeySachkov deleted the private/asachkov/fix-numeric-limit-half branch April 1, 2020 10:23
vmaksimo pushed a commit to vmaksimo/llvm that referenced this pull request Aug 23, 2021
The instruction is for vector types and not for scalar types.
It's a kind of a follow up for:
KhronosGroup/OpenCL-CTS#1002

Signed-off-by: Dmitry Sidorov <dmitry.sidorov@intel.com>

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@1a7f142
vmaksimo pushed a commit to vmaksimo/llvm that referenced this pull request Sep 28, 2021
This reverts commit 1a7f1420db4dc21a9628b85e9c06073c718ebfc4.

The vloada_halfn functions return float vectors:

   floatn vloada_halfn(size_t offset, const __global half *p)
   floatn vloada_halfn(size_t offset, const __local half *p)
   floatn vloada_halfn(size_t offset, const __constant half *p)
   floatn vloada_halfn(size_t offset, const __private half *p)

   For the generic address space:
   floatn vloada_halfn(size_t offset, const half *p)

These functions exist specifically for implementations that lack the
cl_khr_fp16 extension, and so do not include the halfn vector types,
but do include a scalar half type for use as a pointer argument *only*.

Likewise, in the SPIR-V OpenCL Extended Instruction Set, the result type
for vloada_halfn is defined as follows:

   Result Type must be vector(2,3,4,8,16) of float values.

The assertion added to isBuiltinTransToExtInst(...) is therefore
incorrect.

The vloada_halfn SPIR-V OpenCL Extended Instruction Set instructions
that are emitted for calls to these builtins are accepted by the
SPIR-V Validator.

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@2b95385
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.

2 participants