Skip to content

Commit 6854e84

Browse files
committed
[SYCL] Remove redundant build options processing
After 86b0e8d patch extra operations with device images happen before check if it is present in in-memory cache. For applications with small kernels which are executed multiple times noticeable performance degradation is seen for host code. That was done to get build options stored in the kernel image and use them as in-memory cache key. At the same time kernel image (where these options are taken from) is used in cache key so it is reasonable to use only build options which are specified in SYCL API and/or environment variables as separate cache key. Getting build options from kernel image is moved back to build operation which happens only if built program is missed in in-memory cache.
1 parent 7d98ed8 commit 6854e84

File tree

1 file changed

+43
-39
lines changed

1 file changed

+43
-39
lines changed

sycl/source/detail/program_manager/program_manager.cpp

Lines changed: 43 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -367,35 +367,50 @@ RT::PiProgram ProgramManager::getBuiltPIProgram(OSModuleHandle M,
367367
auto GetF = [](const Locked<ProgramCacheT> &LockedCache) -> ProgramCacheT & {
368368
return LockedCache.get();
369369
};
370-
std::string BuildOptions;
371-
if (Prg)
372-
BuildOptions = Prg->get_build_options();
373-
const RTDeviceBinaryImage &Img =
374-
getDeviceImage(M, KSId, Context, Device, JITCompilationIsRequired);
375-
std::string CompileOpts = Img.getCompileOptions();
376-
std::string LinkOpts = Img.getLinkOptions();
377-
pi_device_binary_property isEsimdImage = Img.getProperty("isEsimdImage");
378-
if (!BuildOptions.empty()) {
379-
CompileOpts += " ";
380-
CompileOpts += BuildOptions;
381-
}
382-
if (isEsimdImage && pi::DeviceBinaryProperty(isEsimdImage).asUint32()) {
383-
if (!CompileOpts.empty())
384-
CompileOpts += " ";
385-
CompileOpts += "-vc-codegen";
386-
}
387370

388-
// Build options are overridden if environment variables are present
389-
const char *CompileOptsEnv = SYCLConfig<SYCL_PROGRAM_COMPILE_OPTIONS>::get();
371+
std::string CompileOpts;
372+
std::string LinkOpts;
373+
// Build options are overridden if environment variables are present.
374+
// Environment variables are not changed during program lifecycle so it
375+
// is reasonable to use static here to read them only once.
376+
static const char *CompileOptsEnv =
377+
SYCLConfig<SYCL_PROGRAM_COMPILE_OPTIONS>::get();
390378
if (CompileOptsEnv) {
391379
CompileOpts = CompileOptsEnv;
380+
} else { // Use build options only when the environment variable is missed
381+
if (Prg) {
382+
std::string BuildOptions = Prg->get_build_options();
383+
if (!BuildOptions.empty()) {
384+
CompileOpts += " ";
385+
CompileOpts += BuildOptions;
386+
}
387+
}
392388
}
393-
const char *LinkOptsEnv = SYCLConfig<SYCL_PROGRAM_LINK_OPTIONS>::get();
389+
static const char *LinkOptsEnv = SYCLConfig<SYCL_PROGRAM_LINK_OPTIONS>::get();
394390
if (LinkOptsEnv) {
395391
LinkOpts = LinkOptsEnv;
396392
}
397393

398-
auto BuildF = [this, &Context, &Device, Prg, &Img, &CompileOpts, &LinkOpts] {
394+
auto BuildF = [this, &M, &KSId, &Context, &Device, Prg, &CompileOpts,
395+
&LinkOpts, &JITCompilationIsRequired] {
396+
const RTDeviceBinaryImage &Img =
397+
getDeviceImage(M, KSId, Context, Device, JITCompilationIsRequired);
398+
// Update only if compile options are not overwritten by environment
399+
// variable
400+
if (!CompileOptsEnv) {
401+
CompileOpts += Img.getCompileOptions();
402+
pi_device_binary_property isEsimdImage = Img.getProperty("isEsimdImage");
403+
404+
if (isEsimdImage && pi::DeviceBinaryProperty(isEsimdImage).asUint32()) {
405+
if (!CompileOpts.empty())
406+
CompileOpts += " ";
407+
CompileOpts += "-vc-codegen";
408+
}
409+
}
410+
411+
// Update only if link options are not overwritten by environment variable
412+
if (!LinkOptsEnv)
413+
LinkOpts += Img.getLinkOptions();
399414
ContextImplPtr ContextImpl = getSyclObjImpl(Context);
400415
const detail::plugin &Plugin = ContextImpl->getPlugin();
401416
RT::PiProgram NativePrg = createPIProgram(Img, Context, Device);
@@ -820,14 +835,6 @@ ProgramManager::ProgramPtr ProgramManager::build(
820835
}
821836

822837
bool LinkDeviceLibs = (DeviceLibReqMask != 0);
823-
const char *CompileOpts = std::getenv("SYCL_PROGRAM_COMPILE_OPTIONS");
824-
if (!CompileOpts) {
825-
CompileOpts = CompileOptions.c_str();
826-
}
827-
const char *LinkOpts = std::getenv("SYCL_PROGRAM_LINK_OPTIONS");
828-
if (!LinkOpts) {
829-
LinkOpts = LinkOptions.c_str();
830-
}
831838

832839
// TODO: Currently, online linking isn't implemented yet on Level Zero.
833840
// To enable device libraries and unify the behaviors on all backends,
@@ -839,9 +846,8 @@ ProgramManager::ProgramPtr ProgramManager::build(
839846
// TODO: this is a temporary workaround for GPU tests for ESIMD compiler.
840847
// We do not link with other device libraries, because it may fail
841848
// due to unrecognized SPIR-V format of those libraries.
842-
if (std::string(CompileOpts).find(std::string("-cmc")) != std::string::npos ||
843-
std::string(CompileOpts).find(std::string("-vc-codegen")) !=
844-
std::string::npos)
849+
if (CompileOptions.find(std::string("-cmc")) != std::string::npos ||
850+
CompileOptions.find(std::string("-vc-codegen")) != std::string::npos)
845851
LinkDeviceLibs = false;
846852

847853
std::vector<RT::PiProgram> LinkPrograms;
@@ -852,11 +858,9 @@ ProgramManager::ProgramPtr ProgramManager::build(
852858

853859
const detail::plugin &Plugin = Context->getPlugin();
854860
if (LinkPrograms.empty()) {
855-
std::string Opts(CompileOpts);
856-
857861
RT::PiResult Error = Plugin.call_nocheck<PiApiKind::piProgramBuild>(
858-
Program.get(), /*num devices =*/1, &Device, Opts.c_str(), nullptr,
859-
nullptr);
862+
Program.get(), /*num devices =*/1, &Device, CompileOptions.c_str(),
863+
nullptr, nullptr);
860864
if (Error != PI_SUCCESS)
861865
throw compile_program_error(getProgramBuildLog(Program.get(), Context),
862866
Error);
@@ -865,13 +869,13 @@ ProgramManager::ProgramPtr ProgramManager::build(
865869

866870
// Include the main program and compile/link everything together
867871
Plugin.call<PiApiKind::piProgramCompile>(Program.get(), /*num devices =*/1,
868-
&Device, CompileOpts, 0, nullptr,
869-
nullptr, nullptr, nullptr);
872+
&Device, CompileOptions.c_str(), 0,
873+
nullptr, nullptr, nullptr, nullptr);
870874
LinkPrograms.push_back(Program.get());
871875

872876
RT::PiProgram LinkedProg = nullptr;
873877
RT::PiResult Error = Plugin.call_nocheck<PiApiKind::piProgramLink>(
874-
Context->getHandleRef(), /*num devices =*/1, &Device, LinkOpts,
878+
Context->getHandleRef(), /*num devices =*/1, &Device, LinkOptions.c_str(),
875879
LinkPrograms.size(), LinkPrograms.data(), nullptr, nullptr, &LinkedProg);
876880

877881
// Link program call returns a new program object if all parameters are valid,

0 commit comments

Comments
 (0)