Skip to content

[SYCL] Move devicelib assert wrapper to header files #1398

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1218,9 +1218,14 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
getToolChain().AddCudaIncludeArgs(Args, CmdArgs);

if (JA.isOffloading(Action::OFK_SYCL) ||
Args.hasArg(options::OPT_fsycl_device_only))
Args.hasArg(options::OPT_fsycl_device_only)) {
toolchains::SYCLToolChain::AddSYCLIncludeArgs(D, Args, CmdArgs);

if (getToolChain().getTriple().isSPIR()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make sure, would:

Suggested change
if (getToolChain().getTriple().isSPIR()) {
if (JA.isDeviceOffloading()) {

be unacceptable because DPC++ NVPTX compilations would also pass the condition?

toolchains::SYCLToolChain::AddDevicelibIncludeArgs(CmdArgs);
}
Comment on lines +1224 to +1226
Copy link
Contributor

@AGindinson AGindinson Apr 21, 2020

Choose a reason for hiding this comment

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

A minor suggestion would be to make this logic part of AddSYCLIncludeArgs() (and get rid of the separate AddDevicelibIncludeArgs())

}

// If we are offloading to a target via OpenMP we need to include the
// openmp_wrappers folder which contains alternative system headers.
if (JA.isDeviceOffloading(Action::OFK_OpenMP) &&
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Driver/ToolChains/SYCL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,12 @@ void SYCLToolChain::AddSYCLIncludeArgs(const clang::driver::Driver &Driver,
CC1Args.push_back(DriverArgs.MakeArgString(P));
}

void SYCLToolChain::AddDevicelibIncludeArgs(ArgStringList &CC1Args) {
// Assume that clang system include path is added elsewhere
CC1Args.push_back("-include");
CC1Args.push_back("devicelib/__devicelib_assert.h");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be included somewhere in SYCL headers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The point is to support compilation when SYCL headers are not included, e.g. when SYCL_EXTERNAL is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1.
The idea here is the compiler to provide additional functionality for SPIR target through the functions declared in this header.
Current implementation is not quite implements this idea - it includes the header for SYCL mode rather then SPIR target, so we probably have to skip this include for non-SPIR targets (e.g. nvidia).

Copy link
Contributor

Choose a reason for hiding this comment

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

The point is to support compilation when SYCL headers are not included

Compilation of what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The point is to support compilation when SYCL headers are not included

Compilation of what?

Compilation of source code that do not #include SYCL headers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Current implementation is not quite implements this idea - it includes the header for SYCL mode rather then SPIR target, so we probably have to skip this include for non-SPIR targets (e.g. nvidia).

The last patch includes the header when SYCL language is compiled for SPIR target. Non-SPIR targets should not be affected.

Copy link
Contributor

Choose a reason for hiding this comment

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

Andrew, can we make this include agnostic to compilation mode/language and include it for all compilations for SPIR target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it makes sense - sure. I'm concerned that this feature will be enabled for languages where it doesn't make sense (for OpenCL, in particular).

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you think it doesn't make sense for OpenCL?

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 think this is ultimately for OpenCL folks to decide. Right now this extension is not documented to work for OpenCL.

}

void SYCLToolChain::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
ArgStringList &CC1Args) const {
HostTC.AddClangSystemIncludeArgs(DriverArgs, CC1Args);
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Driver/ToolChains/SYCL.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ class LLVM_LIBRARY_VISIBILITY SYCLToolChain : public ToolChain {
static void AddSYCLIncludeArgs(const clang::driver::Driver &Driver,
const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args);
static void AddDevicelibIncludeArgs(llvm::opt::ArgStringList &CC1Args);
void AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const override;
void AddClangCXXStdlibIncludeArgs(
Expand Down
13 changes: 12 additions & 1 deletion clang/lib/Headers/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ set(openmp_wrapper_files
openmp_wrappers/new
)

set(devicelib_wrapper_files
devicelib/__devicelib_assert.h
devicelib/__devicelib.h
devicelib/spirv_vars.h
)

set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)
set(out_files)
set(generated_files)
Expand Down Expand Up @@ -175,7 +181,7 @@ endfunction(clang_generate_header)


# Copy header files from the source directory to the build directory
foreach( f ${files} ${cuda_wrapper_files} ${ppc_wrapper_files} ${openmp_wrapper_files})
foreach( f ${files} ${cuda_wrapper_files} ${ppc_wrapper_files} ${openmp_wrapper_files} ${devicelib_wrapper_files})
copy_header_to_output_dir(${CMAKE_CURRENT_SOURCE_DIR} ${f})
endforeach( f )

Expand Down Expand Up @@ -218,6 +224,11 @@ install(
DESTINATION ${header_install_dir}/openmp_wrappers
COMPONENT clang-resource-headers)

install(
FILES ${devicelib_wrapper_files}
DESTINATION ${header_install_dir}/devicelib
COMPONENT clang-resource-headers)

if (NOT LLVM_ENABLE_IDE)
add_llvm_install_targets(install-clang-resource-headers
DEPENDS clang-resource-headers
Expand Down
51 changes: 51 additions & 0 deletions clang/lib/Headers/devicelib/__devicelib.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//==------------ devicelib.h - macros for devicelib wrappers ---------------==//
Copy link
Contributor

Choose a reason for hiding this comment

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

This path (i.e. devicelib/) doesn't conflict with anything else, does it?

Copy link
Contributor Author

@asavonic asavonic Apr 21, 2020

Choose a reason for hiding this comment

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

Not sure what do you mean by a "conflict". When installed on a system, this directory resides in $prefix/lib/clang/$version/include directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, then.

//
// 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 __DEVICELIB_H__
#define __DEVICELIB_H__

#ifndef _WIN32
#include <features.h> // for GLIBC macro
#endif

// Devicelib wraps a system C or C++ library
#ifdef __GLIBC__
#define __DEVICELIB_GLIBC__ 1
#elif defined(_WIN32)
#define __DEVICELIB_MSLIBC__ 1
#endif

#ifdef __cplusplus
#define EXTERN_C extern "C"
#else // __cplusplus
#define EXTERN_C
#endif // __cplusplus

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

#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
#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

#ifdef __SYCL_DEVICE_ONLY__
#define __DEVICELIB_DEVICE_ONLY__ 1
#endif

#endif // __DEVICELIB_H__
70 changes: 70 additions & 0 deletions clang/lib/Headers/devicelib/__devicelib_assert.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
//==--------- devicelib-assert.h - wrapper definition for C assert ---------==//
//
// 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 __DEVICELIB_ASSERT_H__
#define __DEVICELIB_ASSERT_H__

#include "__devicelib.h"

#ifdef __DEVICELIB_DEVICE_ONLY__

#include <stddef.h>
#include <stdint.h>

#include "spirv_vars.h"

DEVICE_EXTERN_C
void __devicelib_assert_fail(const char *expr, const char *file, int32_t line,
const char *func, uint64_t gid0, uint64_t gid1,
uint64_t gid2, uint64_t lid0, uint64_t lid1,
uint64_t lid2);

#ifdef __DEVICELIB_GLIBC__
EXTERN_C inline void
__assert_fail(const char *expr, const char *file,
unsigned int line, const char *func) __THROW __attribute__ ((__noreturn__)) {
__devicelib_assert_fail(
expr, file, line, func, __spirv_GlobalInvocationId_x(),
__spirv_GlobalInvocationId_y(), __spirv_GlobalInvocationId_z(),
__spirv_LocalInvocationId_x(), __spirv_LocalInvocationId_y(),
__spirv_LocalInvocationId_z());
}
#elif defined(__DEVICELIB_MSLIBC__)
// Truncates a wide (16 or 32 bit) string (wstr) into an ASCII string (str).
// Any non-ASCII characters are replaced by question mark '?'.
static inline void __truncate_wchar_char_str(const wchar_t *wstr, char *str,
size_t str_size) {
str_size -= 1; // reserve for NULL terminator
while (str_size > 0 && *wstr != L'\0') {
wchar_t w = *wstr++;
*str++ = (w > 0 && w < 127) ? (char)w : '?';
str_size--;
}
*str = '\0';
}

EXTERN_C inline void _wassert(const wchar_t *wexpr, const wchar_t *wfile,
unsigned line) {
// Paths and expressions that are longer than 256 characters are going to be
// truncated.
char file[256];
__truncate_wchar_char_str(wfile, file, sizeof(file));
char expr[256];
__truncate_wchar_char_str(wexpr, expr, sizeof(expr));

__devicelib_assert_fail(
expr, file, line, /*func=*/nullptr, __spirv_GlobalInvocationId_x(),
__spirv_GlobalInvocationId_y(), __spirv_GlobalInvocationId_z(),
__spirv_LocalInvocationId_x(), __spirv_LocalInvocationId_y(),
__spirv_LocalInvocationId_z());
}

#endif

#endif // __DEVICELIB_DEVICE_ONLY__
#endif // __DEVICELIB_ASSERT_H__
147 changes: 147 additions & 0 deletions clang/lib/Headers/devicelib/spirv_vars.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
//==---------- spirv_vars.hpp --- SPIRV variables -------------------------==//
//
// 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 __SPIRV_VARS_H__
#define __SPIRV_VARS_H__

#include <stddef.h>
#include <stdint.h>

#ifdef __cplusplus
#define __EXTERN_C extern "C"
#else
#define __EXTERN_C
#endif

#ifdef __SYCL_DEVICE_ONLY__

#ifdef __SYCL_NVPTX__

SYCL_EXTERNAL size_t __spirv_GlobalInvocationId_x();
SYCL_EXTERNAL size_t __spirv_GlobalInvocationId_y();
SYCL_EXTERNAL size_t __spirv_GlobalInvocationId_z();

SYCL_EXTERNAL size_t __spirv_GlobalSize_x();
SYCL_EXTERNAL size_t __spirv_GlobalSize_y();
SYCL_EXTERNAL size_t __spirv_GlobalSize_z();

SYCL_EXTERNAL size_t __spirv_GlobalOffset_x();
SYCL_EXTERNAL size_t __spirv_GlobalOffset_y();
SYCL_EXTERNAL size_t __spirv_GlobalOffset_z();

SYCL_EXTERNAL size_t __spirv_NumWorkgroups_x();
SYCL_EXTERNAL size_t __spirv_NumWorkgroups_y();
SYCL_EXTERNAL size_t __spirv_NumWorkgroups_z();

SYCL_EXTERNAL size_t __spirv_WorkgroupSize_x();
SYCL_EXTERNAL size_t __spirv_WorkgroupSize_y();
SYCL_EXTERNAL size_t __spirv_WorkgroupSize_z();

SYCL_EXTERNAL size_t __spirv_WorkgroupId_x();
SYCL_EXTERNAL size_t __spirv_WorkgroupId_y();
SYCL_EXTERNAL size_t __spirv_WorkgroupId_z();

SYCL_EXTERNAL size_t __spirv_LocalInvocationId_x();
SYCL_EXTERNAL size_t __spirv_LocalInvocationId_y();
SYCL_EXTERNAL size_t __spirv_LocalInvocationId_z();

#else // __SYCL_NVPTX__

typedef size_t size_t_vec __attribute__((ext_vector_type(3)));
__EXTERN_C const __attribute__((opencl_constant))
size_t_vec __spirv_BuiltInGlobalInvocationId;
__EXTERN_C const __attribute__((opencl_constant))
size_t_vec __spirv_BuiltInGlobalSize;
__EXTERN_C const __attribute__((opencl_constant))
size_t_vec __spirv_BuiltInGlobalOffset;
__EXTERN_C const __attribute__((opencl_constant))
size_t_vec __spirv_BuiltInNumWorkgroups;
__EXTERN_C const __attribute__((opencl_constant))
size_t_vec __spirv_BuiltInWorkgroupSize;
__EXTERN_C const __attribute__((opencl_constant))
size_t_vec __spirv_BuiltInWorkgroupId;
__EXTERN_C const __attribute__((opencl_constant))
size_t_vec __spirv_BuiltInLocalInvocationId;

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

SYCL_EXTERNAL inline size_t __spirv_GlobalSize_x() {
return __spirv_BuiltInGlobalSize.x;
}
SYCL_EXTERNAL inline size_t __spirv_GlobalSize_y() {
return __spirv_BuiltInGlobalSize.y;
}
SYCL_EXTERNAL inline size_t __spirv_GlobalSize_z() {
return __spirv_BuiltInGlobalSize.z;
}

SYCL_EXTERNAL inline size_t __spirv_GlobalOffset_x() {
return __spirv_BuiltInGlobalOffset.x;
}
SYCL_EXTERNAL inline size_t __spirv_GlobalOffset_y() {
return __spirv_BuiltInGlobalOffset.y;
}
SYCL_EXTERNAL inline size_t __spirv_GlobalOffset_z() {
return __spirv_BuiltInGlobalOffset.z;
}

SYCL_EXTERNAL inline size_t __spirv_NumWorkgroups_x() {
return __spirv_BuiltInNumWorkgroups.x;
}
SYCL_EXTERNAL inline size_t __spirv_NumWorkgroups_y() {
return __spirv_BuiltInNumWorkgroups.y;
}
SYCL_EXTERNAL inline size_t __spirv_NumWorkgroups_z() {
return __spirv_BuiltInNumWorkgroups.z;
}

SYCL_EXTERNAL inline size_t __spirv_WorkgroupSize_x() {
return __spirv_BuiltInWorkgroupSize.x;
}
SYCL_EXTERNAL inline size_t __spirv_WorkgroupSize_y() {
return __spirv_BuiltInWorkgroupSize.y;
}
SYCL_EXTERNAL inline size_t __spirv_WorkgroupSize_z() {
return __spirv_BuiltInWorkgroupSize.z;
}

SYCL_EXTERNAL inline size_t __spirv_WorkgroupId_x() {
return __spirv_BuiltInWorkgroupId.x;
}
SYCL_EXTERNAL inline size_t __spirv_WorkgroupId_y() {
return __spirv_BuiltInWorkgroupId.y;
}
SYCL_EXTERNAL inline size_t __spirv_WorkgroupId_z() {
return __spirv_BuiltInWorkgroupId.z;
}

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

#endif // __SYCL_NVPTX__

#endif // __SYCL_DEVICE_ONLY__

#undef __EXTERN_C

#endif // __SPIRV_VARS_H__
3 changes: 3 additions & 0 deletions clang/test/Driver/sycl-device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
// RUN: | FileCheck -check-prefix=CHECK-SYCL-DEV %s
// CHECK-SYCL-DEV: "-fsycl-is-device"{{.*}} "-internal-isystem" "{{.*}}bin{{[/\\]+}}..{{[/\\]+}}include{{[/\\]+}}sycl"

/// Check that devicelib include path is added
// CHECK-SYCL-DEV: "-include" "devicelib{{[/\\]}}__devicelib_assert.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// CHECK-SYCL-DEV: "-include" "devicelib{{[/\\]}}__devicelib_assert.h"
// CHECK-SYCL-DEV: "-fsycl-is-device"{{.*}} "-include" "devicelib{{[/\\]}}__devicelib_assert.h"


Comment on lines +11 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this have separate RUN lines, with and without -fsycl-device-only?

/// Check that "-Wno-sycl-strict" is set on compiler invocation with "-fsycl"
/// or "-fsycl-device-only" or both:
// RUN: %clang -### -fsycl %s 2>&1 \
Expand Down
26 changes: 1 addition & 25 deletions libdevice/cmake/modules/SYCLLibdevice.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,6 @@ set(compile_opts
-sycl-std=2017
)

if (WIN32)
set(devicelib-obj-file ${binary_dir}/libsycl-msvc.o)
add_custom_command(OUTPUT ${devicelib-obj-file}
COMMAND ${clang} -fsycl -c
${compile_opts}
${CMAKE_CURRENT_SOURCE_DIR}/msvc_wrapper.cpp
-o ${devicelib-obj-file}
MAIN_DEPENDENCY msvc_wrapper.cpp
DEPENDS wrapper.h device.h clang
VERBATIM)
else()
set(devicelib-obj-file ${binary_dir}/libsycl-glibc.o)
add_custom_command(OUTPUT ${devicelib-obj-file}
COMMAND ${clang} -fsycl -c
${compile_opts}
${CMAKE_CURRENT_SOURCE_DIR}/glibc_wrapper.cpp
-o ${devicelib-obj-file}
MAIN_DEPENDENCY glibc_wrapper.cpp
DEPENDS wrapper.h device.h clang
VERBATIM)
endif()

set(devicelib-obj-complex ${binary_dir}/libsycl-complex.o)
add_custom_command(OUTPUT ${devicelib-obj-complex}
COMMAND ${clang} -fsycl -c
Expand Down Expand Up @@ -126,7 +104,6 @@ add_custom_command(OUTPUT ${binary_dir}/libsycl-fallback-cmath-fp64.spv
VERBATIM)

add_custom_target(libsycldevice-obj DEPENDS
${devicelib-obj-file}
${devicelib-obj-complex}
${devicelib-obj-complex-fp64}
${devicelib-obj-cmath}
Expand All @@ -149,8 +126,7 @@ else()
set(install_dest lib${LLVM_LIBDIR_SUFFIX})
endif()

install(FILES ${devicelib-obj-file}
${binary_dir}/libsycl-fallback-cassert.spv
install(FILES ${binary_dir}/libsycl-fallback-cassert.spv
${devicelib-obj-complex}
${binary_dir}/libsycl-fallback-complex.spv
${devicelib-obj-complex-fp64}
Expand Down
Loading