Skip to content

Guard libdevice code with __SPIR__. #1672

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.

Sign up for GitHub

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 7 commits into from
May 27, 2020
Merged

Guard libdevice code with __SPIR__. #1672

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4a415da
Guard libdevice code with __SPIR__.
May 1, 2020
0084298
Added build dependencies and cleaned up spirv_vars header file.
May 5, 2020
a1d3b3d
Replaced __SPIR_DEVICE_ONLY__ with __SPIR_TARGET_ONLY__.
May 7, 2020
5cb1bcc
clang-format
May 11, 2020
fdb931b
Cleared spirv_vars.hpp dependence.
May 12, 2020
3b06361
Test for predefined SPIR macros.
May 13, 2020
59882f0
Got rid of __SPIR_TARGET_ONLY__.
May 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/lib/Basic/Targets/SPIR.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ void SPIRTargetInfo::getTargetDefines(const LangOptions &Opts,

void SPIR32TargetInfo::getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const {
SPIRTargetInfo::getTargetDefines(Opts, Builder);
DefineStd(Builder, "SPIR32", Opts);
}

void SPIR64TargetInfo::getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const {
SPIRTargetInfo::getTargetDefines(Opts, Builder);
DefineStd(Builder, "SPIR64", Opts);
}
6 changes: 3 additions & 3 deletions libdevice/cmake/modules/SYCLLibdevice.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ if (WIN32)
${CMAKE_CURRENT_SOURCE_DIR}/msvc_wrapper.cpp
-o ${devicelib-obj-file}
MAIN_DEPENDENCY msvc_wrapper.cpp
DEPENDS wrapper.h device.h clang
DEPENDS wrapper.h device.h spirv_vars.h clang
VERBATIM)
else()
set(devicelib-obj-file ${binary_dir}/libsycl-glibc.o)
Expand All @@ -36,7 +36,7 @@ else()
${CMAKE_CURRENT_SOURCE_DIR}/glibc_wrapper.cpp
-o ${devicelib-obj-file}
MAIN_DEPENDENCY glibc_wrapper.cpp
DEPENDS wrapper.h device.h clang
DEPENDS wrapper.h device.h spirv_vars.h clang
VERBATIM)
endif()

Expand Down Expand Up @@ -86,7 +86,7 @@ add_custom_command(OUTPUT ${binary_dir}/libsycl-fallback-cassert.spv
${CMAKE_CURRENT_SOURCE_DIR}/fallback-cassert.cpp
-o ${binary_dir}/libsycl-fallback-cassert.spv
MAIN_DEPENDENCY fallback-cassert.cpp
DEPENDS wrapper.h device.h clang llvm-spirv
DEPENDS wrapper.h device.h clang spirv_vars.h llvm-spirv
VERBATIM)

add_custom_command(OUTPUT ${binary_dir}/libsycl-fallback-complex.spv
Expand Down
6 changes: 4 additions & 2 deletions libdevice/cmath_wrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
//
//===----------------------------------------------------------------------===//

#include "device.h"
#include "device_math.h"

#if __SPIR_TARGET_ONLY__

DEVICE_EXTERN_C
float scalbnf(float x, int n) { return __devicelib_scalbnf(x, n); }

Expand Down Expand Up @@ -367,4 +368,5 @@ float _FSinh(float x, float y) { // compute y * sinh(x), |y| <= 1
return neg ? -x : x;
}
}
#endif
#endif // defined(_WIN32)
#endif // __SPIR_TARGET_ONLY__
6 changes: 4 additions & 2 deletions libdevice/cmath_wrapper_fp64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
//
//===----------------------------------------------------------------------===//

#include "device.h"
#include "device_math.h"

#if __SPIR_TARGET_ONLY__

// All exported functions in math and complex device libraries are weak
// reference. If users provide their own math or complex functions(with
// the prototype), functions in device libraries will be ignored and
Expand Down Expand Up @@ -397,4 +398,5 @@ double _Sinh(double x, double y) { // compute y * sinh(x), |y| <= 1
return neg ? -x : x;
}
}
#endif
#endif // defined(_WIN32)
#endif // __SPIR_TARGET_ONLY__
4 changes: 3 additions & 1 deletion libdevice/complex_wrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
//
//===----------------------------------------------------------------------===//

#include "device.h"
#include "device_complex.h"

#if __SPIR_TARGET_ONLY__

DEVICE_EXTERN_C
float cimagf(float __complex__ z) { return __devicelib_cimagf(z); }

Expand Down Expand Up @@ -98,3 +99,4 @@ DEVICE_EXTERN_C
float __complex__ __divsc3(float __a, float __b, float __c, float __d) {
return __devicelib___divsc3(__a, __b, __c, __d);
}
#endif // __SPIR_TARGET_ONLY__
4 changes: 3 additions & 1 deletion libdevice/complex_wrapper_fp64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
//
//===----------------------------------------------------------------------===//

#include "device.h"
#include "device_complex.h"

#if __SPIR_TARGET_ONLY__

DEVICE_EXTERN_C
double cimag(double __complex__ z) { return __devicelib_cimag(z); }

Expand Down Expand Up @@ -98,3 +99,4 @@ DEVICE_EXTERN_C
double __complex__ __divdc3(double __a, double __b, double __c, double __d) {
return __devicelib___divdc3(__a, __b, __c, __d);
}
#endif // __SPIR_TARGET_ONLY__
20 changes: 6 additions & 14 deletions libdevice/device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,18 @@
#define EXTERN_C
#endif // __cplusplus

#ifdef CL_SYCL_LANGUAGE_VERSION
#ifndef SYCL_EXTERNAL
#define SYCL_EXTERNAL
#endif // SYCL_EXTERNAL

#if __SPIR__
#define __SPIR_TARGET_ONLY__ 1
#ifdef __SYCL_DEVICE_ONLY__
#define DEVICE_EXTERNAL SYCL_EXTERNAL __attribute__((weak))
#else // __SYCL_DEVICE_ONLY__
#define DEVICE_EXTERNAL static
#undef EXTERN_C
#define EXTERN_C
#define DEVICE_EXTERNAL __attribute__((weak))
#endif // __SYCL_DEVICE_ONLY__
#else // CL_SYCL_LANGUAGE_VERSION
#define DEVICE_EXTERNAL
#endif // CL_SYCL_LANGUAGE_VERSION

#define DEVICE_EXTERN_C DEVICE_EXTERNAL EXTERN_C

// We need the following header to ensure the definition of all spirv variables
// required by the wrapper libraries.
#include "spirv_vars.hpp"
#else // __SPIR__
#define __SPIR_TARGET_ONLY__ 0
Copy link
Contributor

@asavonic asavonic May 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Is this macro needed for non-spir targets? Can it be defined only for spir?

Copy link
Contributor

Choose a reason for hiding this comment

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

One more question: why can't just use __SPIR__?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this macro needed for non-spir targets? Can it be defined only for spir?

I want to make sure that this header file is the only place to control it, so I want to have this macro defined in all compilations. If it is (accidentally) redefined, then the build will break making the issue evident. This is a devicelib internal macro that I want to have full control over.

Copy link
Contributor Author

@vzakhari vzakhari May 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

One more question: why can't just use __SPIR__?

It makes any future manipulations with the implementation guard simpler. Not that I expect any further modifications, but I already did it once (when I removed __SYCL_DEVICE_ONLY__), and it required changes in all files.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it is (accidentally) redefined, then the build will break making the issue evident.

I don't know if it is a real issue. Compiler does not define this macro, users do not define it either. Header defines it in a single place surrounded by include guards.

Macros such as this are usually not defined on "other" platforms. This way __SYCL_DEVICE_ONLY__ is not defined for host compilation (as opposed to be defined to 0) and __X86_64__ is not defined for arm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not worried about users. I am worried about developers :) __SPIR_TARGET_ONLY__ kind of invades onto FE's territory. If at any point it is end up being defined for whatever reason, I want to be aware of it.

If you do not like #define __SPIR_TARGET_ONLY__ 0, I can replace it with #undef __SPIR_TARGET_ONLY__ in the else clause, but I want to have the controlled behavior in all compilations.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not choose a unique prefix, such as __DEVICELIB_DEVICE_ONLY__?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no such thing as unique name, unless it is already defined by FE.

I think we waste too much time discussing this. How about I get rid of __SPIR_TARGET_ONLY__ and use just __SPIR__ instead? I believe everybody agrees with that, am I right?

#endif // __SPIR__

#endif // __LIBDEVICE_DEVICE_H__
3 changes: 3 additions & 0 deletions libdevice/device_complex.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

#include "device.h"

#if __SPIR_TARGET_ONLY__

// TODO: This needs to be more robust.
// clang doesn't recognize the c11 CMPLX macro, but it does have
// its own syntax extension for initializing a complex as a struct.
Expand Down Expand Up @@ -163,4 +165,5 @@ double __complex__ __devicelib___divdc3(double a, double b, double c, double d);

DEVICE_EXTERN_C
float __complex__ __devicelib___divsc3(float a, float b, float c, float d);
#endif // __SPIR_TARGET_ONLY__
#endif // __LIBDEVICE_DEVICE_COMPLEX_H_
3 changes: 3 additions & 0 deletions libdevice/device_math.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

#include "device.h"

#if __SPIR_TARGET_ONLY__

DEVICE_EXTERN_C
double __devicelib_log(double x);

Expand Down Expand Up @@ -247,4 +249,5 @@ float __devicelib_logbf(float x);

DEVICE_EXTERN_C
float __devicelib_scalbnf(float x, int n);
#endif // __SPIR_TARGET_ONLY__
#endif // __LIBDEVICE_DEVICE_MATH_H__
2 changes: 2 additions & 0 deletions libdevice/fallback-cassert.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "wrapper.h"

#if __SPIR_TARGET_ONLY__
static const __attribute__((opencl_constant)) char assert_fmt[] =
"%s:%d: %s: global id: [%lu,%lu,%lu], local id: [%lu,%lu,%lu] "
"Assertion `%s` failed.\n";
Expand All @@ -30,3 +31,4 @@ DEVICE_EXTERN_C void __devicelib_assert_fail(const char *expr, const char *file,
// volatile int *die = (int *)0x0;
// *die = 0xdead;
}
#endif // __SPIR_TARGET_ONLY__
3 changes: 3 additions & 0 deletions libdevice/fallback-cmath-fp64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#include "device_math.h"

#if __SPIR_TARGET_ONLY__

DEVICE_EXTERN_C
double __devicelib_log(double x) { return __spirv_ocl_log(x); }

Expand Down Expand Up @@ -138,3 +140,4 @@ double __devicelib_asinh(double x) { return __spirv_ocl_asinh(x); }

DEVICE_EXTERN_C
double __devicelib_atanh(double x) { return __spirv_ocl_atanh(x); }
#endif // __SPIR_TARGET_ONLY__
4 changes: 4 additions & 0 deletions libdevice/fallback-cmath.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#include "device_math.h"

#if __SPIR_TARGET_ONLY__

DEVICE_EXTERN_C
float __devicelib_scalbnf(float x, int n) { return __spirv_ocl_ldexp(x, n); }

Expand Down Expand Up @@ -139,3 +141,5 @@ float __devicelib_asinhf(float x) { return __spirv_ocl_asinh(x); }

DEVICE_EXTERN_C
float __devicelib_atanhf(float x) { return __spirv_ocl_atanh(x); }

#endif // __SPIR_TARGET_ONLY__
3 changes: 3 additions & 0 deletions libdevice/fallback-complex-fp64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#include "device_complex.h"
#include "device_math.h"

#if __SPIR_TARGET_ONLY__
#include <cmath>

DEVICE_EXTERN_C
Expand Down Expand Up @@ -423,3 +425,4 @@ double __complex__ __devicelib_catan(double __complex__ z) {
__devicelib_catanh(CMPLX(-__devicelib_cimag(z), __devicelib_creal(z)));
return CMPLX(__devicelib_cimag(w), -__devicelib_creal(w));
}
#endif // __SPIR_TARGET_ONLY__
3 changes: 3 additions & 0 deletions libdevice/fallback-complex.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#include "device_complex.h"
#include "device_math.h"

#if __SPIR_TARGET_ONLY__
#include <cmath>

DEVICE_EXTERN_C
Expand Down Expand Up @@ -427,3 +429,4 @@ float __complex__ __devicelib_catanf(float __complex__ z) {
CMPLXF(-__devicelib_cimagf(z), __devicelib_crealf(z)));
return CMPLXF(__devicelib_cimagf(w), -__devicelib_crealf(w));
}
#endif // __SPIR_TARGET_ONLY__
3 changes: 2 additions & 1 deletion libdevice/glibc_wrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
//
//===----------------------------------------------------------------------===//

#include "device.h"
#include "wrapper.h"

#if __SPIR_TARGET_ONLY__
DEVICE_EXTERN_C
void __assert_fail(const char *expr, const char *file, unsigned int line,
const char *func) {
Expand All @@ -18,3 +18,4 @@ void __assert_fail(const char *expr, const char *file, unsigned int line,
__spirv_LocalInvocationId_x(), __spirv_LocalInvocationId_y(),
__spirv_LocalInvocationId_z());
}
#endif // __SPIR_TARGET_ONLY__
3 changes: 2 additions & 1 deletion libdevice/msvc_wrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
//
//===----------------------------------------------------------------------===//

#include "device.h"
#include "wrapper.h"

#if __SPIR_TARGET_ONLY__
// Truncates a wide (16 or 32 bit) string (wstr) into an ASCII string (str).
// Any non-ASCII characters are replaced by question mark '?'.
static void __truncate_wchar_char_str(const wchar_t *wstr, char *str,
Expand Down Expand Up @@ -37,3 +37,4 @@ void _wassert(const wchar_t *wexpr, const wchar_t *wfile, unsigned line) {
__spirv_LocalInvocationId_x(), __spirv_LocalInvocationId_y(),
__spirv_LocalInvocationId_z());
}
#endif // __SPIR_TARGET_ONLY__
44 changes: 44 additions & 0 deletions libdevice/spirv_vars.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//==---------- spirv_vars.h --- SPIRV variables --------------*- C++ -*-----==//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef __LIBDEVICE_SPIRV_VARS_H
#define __LIBDEVICE_SPIRV_VARS_H

#include "device.h"

#if __SPIR_TARGET_ONLY__

#include <cstddef>
#include <cstdint>

typedef size_t size_t_vec __attribute__((ext_vector_type(3)));
extern "C" const size_t_vec __spirv_BuiltInGlobalInvocationId;
extern "C" const size_t_vec __spirv_BuiltInLocalInvocationId;

DEVICE_EXTERNAL inline size_t __spirv_GlobalInvocationId_x() {
return __spirv_BuiltInGlobalInvocationId.x;
}
DEVICE_EXTERNAL inline size_t __spirv_GlobalInvocationId_y() {
return __spirv_BuiltInGlobalInvocationId.y;
}
DEVICE_EXTERNAL inline size_t __spirv_GlobalInvocationId_z() {
return __spirv_BuiltInGlobalInvocationId.z;
}

DEVICE_EXTERNAL inline size_t __spirv_LocalInvocationId_x() {
return __spirv_BuiltInLocalInvocationId.x;
}
DEVICE_EXTERNAL inline size_t __spirv_LocalInvocationId_y() {
return __spirv_BuiltInLocalInvocationId.y;
}
DEVICE_EXTERNAL inline size_t __spirv_LocalInvocationId_z() {
return __spirv_BuiltInLocalInvocationId.z;
}

#endif // __SPIR_TARGET_ONLY__
#endif // __LIBDEVICE_SPIRV_VARS_H
Loading