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.

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

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

Merged
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 all 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
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 to ensure the definition of all spirv variables
// required by the wrapper libraries.
#include "spirv_vars.hpp"
Copy link
Contributor

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

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

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

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

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

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

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would move this include below the guard.


#ifdef __cplusplus
#define EXTERN_C extern "C"
#else // __cplusplus
Expand Down
102 changes: 102 additions & 0 deletions libdevice/spirv_vars.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
//==---------- 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
//
// ===-------------------------------------------------------------------=== //

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

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_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_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(); } \
template <> size_t get##POSTFIX<1>() { return __spirv_##POSTFIX##_y(); } \
template <> size_t get##POSTFIX<2>() { return __spirv_##POSTFIX##_z(); }

namespace __spirv {

DEFINE_FUNC_ID_TO_XYZ_CONVERTER(GlobalInvocationId);
DEFINE_FUNC_ID_TO_XYZ_CONVERTER(LocalInvocationId);

} // namespace __spirv

#undef DEFINE_FUNC_ID_TO_XYZ_CONVERTER

#define DEFINE_INIT_SIZES(POSTFIX) \
\
template <int Dim, class DstT> struct InitSizesST##POSTFIX; \
\
template <class DstT> struct InitSizesST##POSTFIX<1, DstT> { \
static DstT initSize() { return {get##POSTFIX<0>()}; } \
}; \
\
template <class DstT> struct InitSizesST##POSTFIX<2, DstT> { \
static DstT initSize() { return {get##POSTFIX<1>(), get##POSTFIX<0>()}; } \
}; \
\
template <class DstT> struct InitSizesST##POSTFIX<3, DstT> { \
static DstT initSize() { \
return {get##POSTFIX<2>(), get##POSTFIX<1>(), get##POSTFIX<0>()}; \
} \
}; \
\
template <int Dims, class DstT> static DstT init##POSTFIX() { \
return InitSizesST##POSTFIX<Dims, DstT>::initSize(); \
}

namespace __spirv {

DEFINE_INIT_SIZES(GlobalInvocationId)
DEFINE_INIT_SIZES(LocalInvocationId)

} // namespace __spirv

#undef DEFINE_INIT_SIZES

#endif // __SYCL_DEVICE_ONLY__
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.

Loading