Skip to content
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

[half]Add ifdefs around usages of half #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sbalint98
Copy link

in hipSYCL half is not implemented yet therefore we need to disable the functions that use it

@sbalint98 sbalint98 force-pushed the half branch 4 times, most recently from 146b9e2 to f9a3289 Compare May 21, 2021 12:14
@sbalint98 sbalint98 marked this pull request as ready for review May 21, 2021 13:03
Use DISABLE_HALF_RUTINES, use cmake config to set
@@ -25,6 +25,7 @@
#cmakedefine ENABLE_MKLCPU_BACKEND
#cmakedefine ENABLE_MKLGPU_BACKEND
#cmakedefine ENABLE_NETLIB_BACKEND
#cmakedefine DISABLE_HALF_RUTINES

Choose a reason for hiding this comment

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

typo: routines and also I'm not really a fan of disabling stuff... can we make this a ENABLE.. and default to ON or isn't it possible to default something in this weird file? :)

src/rng/backends/mklcpu/mrg32k3a.cpp Show resolved Hide resolved
@@ -261,7 +261,7 @@ static inline void gemm(cl::sycl::queue &queue, transpose transa, transpose tran
c, ldc);
gemm_postcondition(queue, transa, transb, m, n, k, alpha, a, lda, b, ldb, beta, c, ldc);
}

#ifdef ENABLE_HALF_ROUTINES
Copy link

Choose a reason for hiding this comment

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

Do we even need to ifdef away this interface entirely? Could we just keep the interface and throw an exception if we detect that we want to run a half function on a device that does not support half (which currently is always in hipSYCL)? I think hipSYCL defines the half type to something like a short, so the interface by itself should compile.

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