Skip to content

[SYCL] Ensure proper definition of spirv builtins for SYCL #1393

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
bader merged 7 commits into from
Apr 9, 2020
Merged

[SYCL] Ensure proper definition of spirv builtins for SYCL #1393

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1f34bcd
[SYCL] Ensure proper definition of spirv builtins for SYCL
Mar 25, 2020
377bb1b
[SYCL] Format spirv_vars.hpp
Mar 25, 2020
f124161
Merge remote-tracking branch 'intel-public/sycl' into Alex/spirv-buil…
Mar 26, 2020
91b92ed
[SYCL] Remove downstream changes from llvm-spirv
Mar 26, 2020
41ea7ab
[SYCL] Implement required builtins for libdevice
Mar 30, 2020
253d28a
[SYCL] Reinclude spirv_vars in libdevice
Mar 31, 2020
09930df
Move include within header definition guard
Apr 3, 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
5 changes: 5 additions & 0 deletions libdevice/cmake/modules/SYCLLibdevice.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ endif()

set(clang $<TARGET_FILE:clang>)

set(sycl_spirv_headers ${LLVM_EXTERNAL_SYCL_SOURCE_DIR}/include/CL/__spirv)

set(compile_opts
# suppress an error about SYCL_EXTERNAL being used for
# a function with a raw pointer parameter.
Expand All @@ -16,6 +18,9 @@ set(compile_opts
# Force definition of CL_SYCL_LANGUAGE_VERSION, as long as
# SYCL specific code is guarded by it.
-sycl-std=2017
# We require the spirv_vars.hpp header from the sycl project
# for access to all the required spirv builtin vars
-I${sycl_spirv_headers}
)

if (WIN32)
Expand Down
4 changes: 4 additions & 0 deletions libdevice/device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
#ifndef __LIBDEVICE_DEVICE_H__
#define __LIBDEVICE_DEVICE_H__

// We need the following header provided by the sycl project to ensure
// definition of all spirv variables required by the wrapper libraries.
#include "spirv_vars.hpp"
Copy link
Contributor

@bader bader Mar 26, 2020

Choose a reason for hiding this comment

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

This looks like a broken design.
libdevice should not include sycl library headers.
@vzakhari, recently moved this library out of the sycl project to make them independent (i.e. we should be able to one w/o the other).

Doesn't #1384 help to have all needed definitions?

Copy link
Contributor

@vzakhari vzakhari Mar 26, 2020

Choose a reason for hiding this comment

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

I see that #1384 removed all _spirv* declarations from libdevice/device_math.h, and I do not understand how it passed the testing. @bader, do you know?

I agree that devicelib must not depend on any headers from SYCL project. We are building devicelib in a fork without SYCL support, so I'd rather keep devicelib self-contained.

Copy link
Contributor

@bader bader Mar 26, 2020

Choose a reason for hiding this comment

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

I see that #1384 removed all _spirv* declarations from libdevice/device_math.h, and I do not understand how it passed the testing. @bader, do you know?

I agree that devicelib must not depend on any headers from SYCL project. We are building devicelib in a fork without SYCL support, so I'd rather keep devicelib self-contained.

With this change SPIR-V built-ins are "clang" built-ins i.e. they are recognized by clang w/o forward declarations as they are declared by clang itself.

Copy link
Contributor

@vzakhari vzakhari Mar 26, 2020

Choose a reason for hiding this comment

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

Thank you! I will have to use -fdeclare-spirv-builtins for non-SYCL builds of libdevice.

Copy link
Contributor Author

@Alexander-Johnston Alexander-Johnston Mar 27, 2020

Choose a reason for hiding this comment

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

This looks like a broken design.
libdevice should not include sycl library headers.
@vzakhari, recently moved this library out of the sycl project to make them independent (i.e. we should be able to one w/o the other).

Doesn't #1384 help to have all needed definitions?

This is required because the _devicelib_assert_fail calls in glibc_wrapper.cpp and msvc_wrapper.cpp both require the global and local invocation IDs.

  __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());

The upstream llvm-spirv translator doesn't implement these, so we implemented them in a somewhat broken way in the llvm-spirv translator, see #1166. This patch is triyng to avoid upstreaming the broken changes to the translator by implementing the invocation ID functions (among some others) for non NVPTX inside spirv_vars.hpp, which is why the devicelib has a requirement on the SYCL header.

If you'd prefer, an alternative to the header dependency would be to have another header in the devicelib which implements only the required invocation ID functions.

Copy link
Contributor

@vzakhari vzakhari Mar 27, 2020

Choose a reason for hiding this comment

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

Yes, please make the implementation in a devicelib header. Not that I like it, but I want devicelib to be self-contained, and do not see a better way now.

Copy link
Contributor Author

@Alexander-Johnston Alexander-Johnston Mar 30, 2020

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@vzakhari vzakhari Mar 30, 2020

Choose a reason for hiding this comment

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

Thank you, @Alexander-Johnston! LGTM for libdevice.


#ifdef __cplusplus
#define EXTERN_C extern "C"
#else // __cplusplus
Expand Down
52 changes: 7 additions & 45 deletions llvm-spirv/lib/SPIRV/OCL20ToSPIRV.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,31 +273,11 @@ class OCL20ToSPIRV : public ModulePass, public InstVisitor<OCL20ToSPIRV> {
Module *M;
LLVMContext *Ctx;
unsigned CLVer; /// OpenCL version as major*10+minor
unsigned CLLang; /// OpenCL language, see `spv::SourceLanguage`.
std::set<Value *> ValuesToDelete;

ConstantInt *addInt32(int I) { return getInt32(M, I); }
ConstantInt *addSizet(uint64_t I) { return getSizet(M, I); }

/// Return the index of the id dimension represented by the demangled built-in name.
/// ie. given `__spirv__GlobalInvocationId_x`, return `0`.
Optional<uint64_t> spirvDimensionFromBuiltin(StringRef Name) {
if (!Name.startswith("__spirv_")) {
return {};
}

Optional<uint64_t> Result = {};
if (Name.endswith("_x")) {
Result = 0;
} else if (Name.endswith("_y")) {
Result = 1;
} else if (Name.endswith("_z")) {
Result = 2;
}

return Result;
}

/// Get vector width from OpenCL vload* function name.
SPIRVWord getVecLoadWidth(const std::string &DemangledName) {
SPIRVWord Width = 0;
Expand Down Expand Up @@ -347,8 +327,7 @@ bool OCL20ToSPIRV::runOnModule(Module &Module) {
M = &Module;
Ctx = &M->getContext();
auto Src = getSPIRVSource(&Module);
CLLang = std::get<0>(Src);
if (CLLang != spv::SourceLanguageOpenCL_C && CLLang != spv::SourceLanguageOpenCL_CPP)
if (std::get<0>(Src) != spv::SourceLanguageOpenCL_C)
return false;

CLVer = std::get<1>(Src);
Expand Down Expand Up @@ -1245,18 +1224,9 @@ void OCL20ToSPIRV::transWorkItemBuiltinsToVariables() {
std::vector<Function *> WorkList;
for (auto &I : *M) {
StringRef DemangledName;
auto MangledName = I.getName();
LLVM_DEBUG(dbgs() << "Function mangled name: " << MangledName << '\n');
if (!oclIsBuiltin(MangledName, DemangledName))
if (!oclIsBuiltin(I.getName(), DemangledName))
continue;
LLVM_DEBUG(dbgs() << "Function demangled name: " << DemangledName << '\n');
auto SpirvDimension {spirvDimensionFromBuiltin(DemangledName)};
auto IsSpirvBuiltinWithDimensions {SpirvDimension.hasValue()};
if ((!IsSpirvBuiltinWithDimensions && CLLang == spv::SourceLanguageOpenCL_CPP) ||
(IsSpirvBuiltinWithDimensions && CLLang == spv::SourceLanguageOpenCL_C)) {
// Only transform `__spirv_` builtins in OpenCL C++.
continue;
}
std::string BuiltinVarName;
SPIRVBuiltinVariableKind BVKind;
if (!SPIRSPIRVBuiltinVariableMap::find(DemangledName.str(), &BVKind))
Expand All @@ -1265,15 +1235,11 @@ void OCL20ToSPIRV::transWorkItemBuiltinsToVariables() {
std::string(kSPIRVName::Prefix) + SPIRVBuiltInNameMap::map(BVKind);
LLVM_DEBUG(dbgs() << "builtin variable name: " << BuiltinVarName << '\n');
bool IsVec = I.getFunctionType()->getNumParams() > 0;
Type *GVType = (IsVec || IsSpirvBuiltinWithDimensions) ?
VectorType::get(I.getReturnType(), 3) : I.getReturnType();
// Each of the `__spirv__GlobalInvocationId_*` functions all extract an element of
// the same global variable, so ensure that we only create the global once.
auto BV = M->getOrInsertGlobal(BuiltinVarName, GVType, [&] {
return new GlobalVariable(
*M, GVType, true, GlobalValue::ExternalLinkage, nullptr, BuiltinVarName,
0, GlobalVariable::NotThreadLocal, SPIRAS_Input);
});
Type *GVType =
IsVec ? VectorType::get(I.getReturnType(), 3) : I.getReturnType();
auto BV = new GlobalVariable(*M, GVType, true, GlobalValue::ExternalLinkage,
nullptr, BuiltinVarName, 0,
GlobalVariable::NotThreadLocal, SPIRAS_Input);
std::vector<Instruction *> InstList;
for (auto UI = I.user_begin(), UE = I.user_end(); UI != UE; ++UI) {
auto CI = dyn_cast<CallInst>(*UI);
Expand All @@ -1284,10 +1250,6 @@ void OCL20ToSPIRV::transWorkItemBuiltinsToVariables() {
NewValue =
ExtractElementInst::Create(NewValue, CI->getArgOperand(0), "", CI);
LLVM_DEBUG(dbgs() << *NewValue << '\n');
} else if (IsSpirvBuiltinWithDimensions) {
auto Index = ConstantInt::get(I.getReturnType(), SpirvDimension.getValue(), false);
NewValue = ExtractElementInst::Create(NewValue, Index, "", CI);
LLVM_DEBUG(dbgs() << *NewValue << '\n');
}
NewValue->takeName(CI);
CI->replaceAllUsesWith(NewValue);
Expand Down
30 changes: 0 additions & 30 deletions llvm-spirv/lib/SPIRV/OCLUtil.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,46 +595,16 @@ template <> inline void SPIRVMap<OclExt::Kind, SPIRVCapabilityKind>::init() {
template <>
inline void SPIRVMap<std::string, SPIRVBuiltinVariableKind>::init() {
add("get_work_dim", BuiltInWorkDim);
add("__spirv_GlobalSize_x", BuiltInGlobalSize);
add("__spirv_GlobalSize_y", BuiltInGlobalSize);
add("__spirv_GlobalSize_z", BuiltInGlobalSize);
add("get_global_size", BuiltInGlobalSize);
add("__spirv_GlobalInvocationId_x", BuiltInGlobalInvocationId);
add("__spirv_GlobalInvocationId_y", BuiltInGlobalInvocationId);
add("__spirv_GlobalInvocationId_z", BuiltInGlobalInvocationId);
add("get_global_id", BuiltInGlobalInvocationId);
add("__spirv_GlobalOffset_x", BuiltInGlobalOffset);
add("__spirv_GlobalOffset_y", BuiltInGlobalOffset);
add("__spirv_GlobalOffset_z", BuiltInGlobalOffset);
add("get_global_offset", BuiltInGlobalOffset);
add("__spirv_WorkgroupSize_x", BuiltInWorkgroupSize);
add("__spirv_WorkgroupSize_y", BuiltInWorkgroupSize);
add("__spirv_WorkgroupSize_z", BuiltInWorkgroupSize);
add("get_local_size", BuiltInWorkgroupSize);
add("__spirv_WorkgroupSize_x", BuiltInWorkgroupSize);
add("__spirv_WorkgroupSize_y", BuiltInWorkgroupSize);
add("__spirv_WorkgroupSize_z", BuiltInWorkgroupSize);
add("get_enqueued_local_size", BuiltInEnqueuedWorkgroupSize);
add("__spirv_LocalInvocationId_x", BuiltInLocalInvocationId);
add("__spirv_LocalInvocationId_y", BuiltInLocalInvocationId);
add("__spirv_LocalInvocationId_z", BuiltInLocalInvocationId);
add("get_local_id", BuiltInLocalInvocationId);
add("__spirv_NumWorkgroups_x", BuiltInNumWorkgroups);
add("__spirv_NumWorkgroups_y", BuiltInNumWorkgroups);
add("__spirv_NumWorkgroups_z", BuiltInNumWorkgroups);
add("get_num_groups", BuiltInNumWorkgroups);
add("__spirv_WorkgroupId_x", BuiltInWorkgroupId);
add("__spirv_WorkgroupId_y", BuiltInWorkgroupId);
add("__spirv_WorkgroupId_z", BuiltInWorkgroupId);
add("get_group_id", BuiltInWorkgroupId);
add("__spirv_WorkgroupId_x", BuiltInWorkgroupId);
add("__spirv_WorkgroupId_y", BuiltInWorkgroupId);
add("__spirv_WorkgroupId_z", BuiltInWorkgroupId);
add("get_global_linear_id", BuiltInGlobalLinearId);
add("get_local_linear_id", BuiltInLocalInvocationIndex);
add("__spirv_LocalInvocationId_x", BuiltInLocalInvocationId);
add("__spirv_LocalInvocationId_y", BuiltInLocalInvocationId);
add("__spirv_LocalInvocationId_z", BuiltInLocalInvocationId);
add("get_sub_group_size", BuiltInSubgroupSize);
add("get_max_sub_group_size", BuiltInSubgroupMaxSize);
add("get_num_sub_groups", BuiltInNumSubgroups);
Expand Down
41 changes: 0 additions & 41 deletions llvm-spirv/test/builtin_vars_to_func.ll
View file
Open in desktop

This file was deleted.

42 changes: 0 additions & 42 deletions llvm-spirv/test/builtin_vars_to_func_cpp.ll
View file
Open in desktop

This file was deleted.

113 changes: 107 additions & 6 deletions sycl/include/CL/__spirv/spirv_vars.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,13 @@

#pragma once

#include <cstddef>
#include <cstdint>

#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();
Expand Down Expand Up @@ -38,6 +43,96 @@ 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__

#define DEFINE_FUNC_ID_TO_XYZ_CONVERTER(POSTFIX) \
template <int ID> static inline size_t get##POSTFIX(); \
template <> size_t get##POSTFIX<0>() { return __spirv_##POSTFIX##_x(); } \
Expand All @@ -58,12 +153,18 @@ DEFINE_FUNC_ID_TO_XYZ_CONVERTER(LocalInvocationId);

#undef DEFINE_FUNC_ID_TO_XYZ_CONVERTER

extern "C" const __attribute__((opencl_constant)) uint32_t __spirv_BuiltInSubgroupSize;
extern "C" const __attribute__((opencl_constant)) uint32_t __spirv_BuiltInSubgroupMaxSize;
extern "C" const __attribute__((opencl_constant)) uint32_t __spirv_BuiltInNumSubgroups;
extern "C" const __attribute__((opencl_constant)) uint32_t __spirv_BuiltInNumEnqueuedSubgroups;
extern "C" const __attribute__((opencl_constant)) uint32_t __spirv_BuiltInSubgroupId;
extern "C" const __attribute__((opencl_constant)) uint32_t __spirv_BuiltInSubgroupLocalInvocationId;
extern "C" const __attribute__((opencl_constant))
uint32_t __spirv_BuiltInSubgroupSize;
extern "C" const __attribute__((opencl_constant))
uint32_t __spirv_BuiltInSubgroupMaxSize;
extern "C" const __attribute__((opencl_constant))
uint32_t __spirv_BuiltInNumSubgroups;
extern "C" const __attribute__((opencl_constant))
uint32_t __spirv_BuiltInNumEnqueuedSubgroups;
extern "C" const __attribute__((opencl_constant))
uint32_t __spirv_BuiltInSubgroupId;
extern "C" const __attribute__((opencl_constant))
uint32_t __spirv_BuiltInSubgroupLocalInvocationId;

#define DEFINE_INIT_SIZES(POSTFIX) \
\
Expand Down