Skip to content

[SYCL] Fix build options merge in program manager #3849

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

Merged
merged 2 commits into from
Jun 1, 2021
Merged
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
120 changes: 56 additions & 64 deletions sycl/source/detail/program_manager/program_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,54 @@ RT::PiProgram ProgramManager::createPIProgram(const RTDeviceBinaryImage &Img,

return Res;
}
static void applyOptionsFromImage(std::string &CompileOpts,
std::string &LinkOpts,
const RTDeviceBinaryImage &Img) {
// Build options are overridden if environment variables are present.
// Environment variables are not changed during program lifecycle so it
// is reasonable to use static here to read them only once.
static const char *CompileOptsEnv =
SYCLConfig<SYCL_PROGRAM_COMPILE_OPTIONS>::get();
static const char *LinkOptsEnv = SYCLConfig<SYCL_PROGRAM_LINK_OPTIONS>::get();
// Update only if compile options are not overwritten by environment
// variable
if (!CompileOptsEnv) {
if (!CompileOpts.empty())
CompileOpts += " ";
CompileOpts += Img.getCompileOptions();
}

// The -vc-codegen option is always preserved for ESIMD kernels, regardless
// of the contents SYCL_PROGRAM_COMPILE_OPTIONS environment variable.
pi_device_binary_property isEsimdImage = Img.getProperty("isEsimdImage");
if (isEsimdImage && pi::DeviceBinaryProperty(isEsimdImage).asUint32()) {
if (!CompileOpts.empty())
CompileOpts += " ";
CompileOpts += "-vc-codegen";
}

// Update only if link options are not overwritten by environment variable
if (!LinkOptsEnv)
if (!LinkOpts.empty())
LinkOpts += " ";
LinkOpts += Img.getLinkOptions();
}

static void applyOptionsFromEnvironment(std::string &CompileOpts,
std::string &LinkOpts) {
// Build options are overridden if environment variables are present.
// Environment variables are not changed during program lifecycle so it
// is reasonable to use static here to read them only once.
static const char *CompileOptsEnv =
SYCLConfig<SYCL_PROGRAM_COMPILE_OPTIONS>::get();
if (CompileOptsEnv) {
CompileOpts = CompileOptsEnv;
}
static const char *LinkOptsEnv = SYCLConfig<SYCL_PROGRAM_LINK_OPTIONS>::get();
if (LinkOptsEnv) {
LinkOpts = LinkOptsEnv;
}
}

RT::PiProgram ProgramManager::getBuiltPIProgram(OSModuleHandle M,
const context &Context,
Expand Down Expand Up @@ -374,26 +422,12 @@ RT::PiProgram ProgramManager::getBuiltPIProgram(OSModuleHandle M,

std::string CompileOpts;
std::string LinkOpts;
// Build options are overridden if environment variables are present.
// Environment variables are not changed during program lifecycle so it
// is reasonable to use static here to read them only once.
static const char *CompileOptsEnv =
SYCLConfig<SYCL_PROGRAM_COMPILE_OPTIONS>::get();
if (CompileOptsEnv) {
CompileOpts = CompileOptsEnv;
} else { // Use build options only when the environment variable is missed
if (Prg) {
std::string BuildOptions = Prg->get_build_options();
if (!BuildOptions.empty()) {
CompileOpts += " ";
CompileOpts += BuildOptions;
}
}
}
static const char *LinkOptsEnv = SYCLConfig<SYCL_PROGRAM_LINK_OPTIONS>::get();
if (LinkOptsEnv) {
LinkOpts = LinkOptsEnv;
if (Prg) {
CompileOpts = Prg->get_build_options();
}

applyOptionsFromEnvironment(CompileOpts, LinkOpts);

SerializedObj SpecConsts;
if (Prg)
Prg->stableSerializeSpecConstRegistry(SpecConsts);
Expand All @@ -402,24 +436,8 @@ RT::PiProgram ProgramManager::getBuiltPIProgram(OSModuleHandle M,
&LinkOpts, &JITCompilationIsRequired, SpecConsts] {
const RTDeviceBinaryImage &Img =
getDeviceImage(M, KSId, Context, Device, JITCompilationIsRequired);
// Update only if compile options are not overwritten by environment
// variable
if (!CompileOptsEnv) {
CompileOpts += Img.getCompileOptions();
}

// The -vc-codegen option is always preserved for ESIMD kernels, regardless
// of the contents SYCL_PROGRAM_COMPILE_OPTIONS environment variable.
pi_device_binary_property isEsimdImage = Img.getProperty("isEsimdImage");
if (isEsimdImage && pi::DeviceBinaryProperty(isEsimdImage).asUint32()) {
if (!CompileOpts.empty())
CompileOpts += " ";
CompileOpts += "-vc-codegen";
}

// Update only if link options are not overwritten by environment variable
if (!LinkOptsEnv)
LinkOpts += Img.getLinkOptions();
applyOptionsFromImage(CompileOpts, LinkOpts, Img);
ContextImplPtr ContextImpl = getSyclObjImpl(Context);
const detail::plugin &Plugin = ContextImpl->getPlugin();
RT::PiProgram NativePrg;
Expand Down Expand Up @@ -1516,41 +1534,15 @@ device_image_plain ProgramManager::build(const device_image_plain &DeviceImage,

std::string CompileOpts;
std::string LinkOpts;
// Build options are overridden if environment variables are present.
// Environment variables are not changed during program lifecycle so it
// is reasonable to use static here to read them only once.
static const char *CompileOptsEnv =
SYCLConfig<SYCL_PROGRAM_COMPILE_OPTIONS>::get();
if (CompileOptsEnv)
CompileOpts = CompileOptsEnv;

static const char *LinkOptsEnv = SYCLConfig<SYCL_PROGRAM_LINK_OPTIONS>::get();
if (LinkOptsEnv) {
LinkOpts = LinkOptsEnv;
}
applyOptionsFromEnvironment(CompileOpts, LinkOpts);

const RTDeviceBinaryImage *ImgPtr = InputImpl->get_bin_image_ref();
const RTDeviceBinaryImage &Img = *ImgPtr;

// TODO: Unify this code with getBuiltPIProgram
auto BuildF = [this, &Context, Img, &Devs, &CompileOpts, &LinkOpts,
&InputImpl] {
// Update only if compile options are not overwritten by environment
// variable
if (!CompileOptsEnv) {
CompileOpts += Img.getCompileOptions();
pi_device_binary_property isEsimdImage = Img.getProperty("isEsimdImage");

if (isEsimdImage && pi::DeviceBinaryProperty(isEsimdImage).asUint32()) {
if (!CompileOpts.empty())
CompileOpts += " ";
CompileOpts += "-vc-codegen";
}
}

// Update only if link options are not overwritten by environment variable
if (!LinkOptsEnv)
LinkOpts += Img.getLinkOptions();
applyOptionsFromImage(CompileOpts, LinkOpts, Img);
ContextImplPtr ContextImpl = getSyclObjImpl(Context);
const detail::plugin &Plugin = ContextImpl->getPlugin();

Expand Down
2 changes: 2 additions & 0 deletions sycl/unittests/get_native_interop/test_get_native.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
//
//===----------------------------------------------------------------------===//

#define SYCL2020_DISABLE_DEPRECATION_WARNINGS

#include <CL/sycl.hpp>
#include <CL/sycl/backend/opencl.hpp>
#include <detail/context_impl.hpp>
Expand Down
21 changes: 19 additions & 2 deletions sycl/unittests/helpers/PiImage.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,14 @@ class PiProperty {

private:
void updateNativeType() {
MNative = NativeType{const_cast<char *>(MName.c_str()),
const_cast<char *>(MData.data()), MType, MData.size()};
if (MType == PI_PROPERTY_TYPE_UINT32) {
MNative = NativeType{const_cast<char *>(MName.c_str()), nullptr, MType,
*((uint32_t *)MData.data())};
} else {
MNative =
NativeType{const_cast<char *>(MName.c_str()),
const_cast<char *>(MData.data()), MType, MData.size()};
}
}
std::string MName;
std::vector<char> MData;
Expand Down Expand Up @@ -366,6 +372,17 @@ void addSpecConstants(PiArray<PiProperty> SpecConstants,
std::move(DefaultValues));
}

/// Utility function to add ESIMD kernel flag to property set.
void addESIMDFlag(PiPropertySet &Props) {
std::vector<char> ValData(sizeof(uint32_t));
ValData[0] = 1;
PiProperty Prop{"isEsimdImage", ValData, PI_PROPERTY_TYPE_UINT32};

PiArray<PiProperty> Value{std::move(Prop)};

Props.insert(__SYCL_PI_PROPERTY_SET_SYCL_MISC_PROP, std::move(Value));
}

/// Utility function to generate offload entries for kernels without arguments.
PiArray<PiOffloadEntry>
makeEmptyKernels(std::initializer_list<std::string> KernelNames) {
Expand Down
3 changes: 2 additions & 1 deletion sycl/unittests/misc/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
set(sycl_lib_dir $<TARGET_FILE_DIR:sycl>)
add_definitions(-DSYCL_LIB_DIR="${sycl_lib_dir}")
add_sycl_unittest(MiscTests SHARED
OsUtils.cpp
CircularBuffer.cpp
KernelBuildOptions.cpp
OsUtils.cpp
)
Loading