Skip to content

Commit

Permalink
[SYCL][NFC] Remove dead arg and check (#16534)
Browse files Browse the repository at this point in the history
The JITCompilationIsRequired in a selection of build functions default
to false and are never set by any callers. Given this, the argument can
be removed. As a side-effect of this, the CheckJITCompilationForImage
function will return immediately if the JITCompilationIsRequired was
false, and since it always was the function is obsolete and can be
removed.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
  • Loading branch information
steffenlarsen authored Jan 9, 2025
1 parent 4d3ffae commit cd30a59
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 42 deletions.
41 changes: 7 additions & 34 deletions sycl/source/detail/program_manager/program_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -781,8 +781,7 @@ CheckAndDecompressImage([[maybe_unused]] RTDeviceBinaryImage *Img) {
// its ref count incremented.
ur_program_handle_t ProgramManager::getBuiltURProgram(
const ContextImplPtr &ContextImpl, const DeviceImplPtr &DeviceImpl,
const std::string &KernelName, const NDRDescT &NDRDesc,
bool JITCompilationIsRequired) {
const std::string &KernelName, const NDRDescT &NDRDesc) {
// Check if we can optimize program builds for sub-devices by using a program
// built for the root device
DeviceImplPtr RootDevImpl = DeviceImpl;
Expand All @@ -803,8 +802,7 @@ ur_program_handle_t ProgramManager::getBuiltURProgram(
auto Context = createSyclObjFromImpl<context>(ContextImpl);
auto Device = createSyclObjFromImpl<device>(
MustBuildOnSubdevice == true ? DeviceImpl : RootDevImpl);
const RTDeviceBinaryImage &Img =
getDeviceImage(KernelName, Context, Device, JITCompilationIsRequired);
const RTDeviceBinaryImage &Img = getDeviceImage(KernelName, Context, Device);

// Check that device supports all aspects used by the kernel
if (auto exception = checkDevSupportDeviceRequirements(Device, Img, NDRDesc))
Expand Down Expand Up @@ -1403,23 +1401,6 @@ ProgramManager::ProgramManager()
}
}

void CheckJITCompilationForImage(const RTDeviceBinaryImage *const &Image,
bool JITCompilationIsRequired) {
if (!JITCompilationIsRequired)
return;
// If the image is already compiled with AOT, throw an exception.
const sycl_device_binary_struct &RawImg = Image->getRawData();
if ((strcmp(RawImg.DeviceTargetSpec,
__SYCL_DEVICE_BINARY_TARGET_SPIRV64_X86_64) == 0) ||
(strcmp(RawImg.DeviceTargetSpec,
__SYCL_DEVICE_BINARY_TARGET_SPIRV64_GEN) == 0) ||
(strcmp(RawImg.DeviceTargetSpec,
__SYCL_DEVICE_BINARY_TARGET_SPIRV64_FPGA) == 0)) {
throw sycl::exception(sycl::errc::feature_not_supported,
"Recompiling AOT image is not supported");
}
}

const char *getArchName(const device &Device) {
namespace syclex = sycl::ext::oneapi::experimental;
auto Arch = getSyclObjImpl(Device)->getDeviceArch();
Expand Down Expand Up @@ -1481,13 +1462,11 @@ RTDeviceBinaryImage *getBinImageFromMultiMap(

RTDeviceBinaryImage &
ProgramManager::getDeviceImage(const std::string &KernelName,
const context &Context, const device &Device,
bool JITCompilationIsRequired) {
const context &Context, const device &Device) {
if constexpr (DbgProgMgr > 0) {
std::cerr << ">>> ProgramManager::getDeviceImage(\"" << KernelName << "\", "
<< getSyclObjImpl(Context).get() << ", "
<< getSyclObjImpl(Device).get() << ", "
<< JITCompilationIsRequired << ")\n";
<< getSyclObjImpl(Device).get() << ")\n";

std::cerr << "available device images:\n";
debugPrintBinaryImages();
Expand All @@ -1497,7 +1476,7 @@ ProgramManager::getDeviceImage(const std::string &KernelName,
assert(m_SpvFileImage);
return getDeviceImage(
std::unordered_set<RTDeviceBinaryImage *>({m_SpvFileImage.get()}),
Context, Device, JITCompilationIsRequired);
Context, Device);
}

RTDeviceBinaryImage *Img = nullptr;
Expand All @@ -1517,8 +1496,6 @@ ProgramManager::getDeviceImage(const std::string &KernelName,
CheckAndDecompressImage(Img);

if (Img) {
CheckJITCompilationForImage(Img, JITCompilationIsRequired);

if constexpr (DbgProgMgr > 0) {
std::cerr << "selected device image: " << &Img->getRawData() << "\n";
Img->print();
Expand All @@ -1532,15 +1509,13 @@ ProgramManager::getDeviceImage(const std::string &KernelName,

RTDeviceBinaryImage &ProgramManager::getDeviceImage(
const std::unordered_set<RTDeviceBinaryImage *> &ImageSet,
const context &Context, const device &Device,
bool JITCompilationIsRequired) {
const context &Context, const device &Device) {
assert(ImageSet.size() > 0);

if constexpr (DbgProgMgr > 0) {
std::cerr << ">>> ProgramManager::getDeviceImage(Custom SPV file "
<< getSyclObjImpl(Context).get() << ", "
<< getSyclObjImpl(Device).get() << ", "
<< JITCompilationIsRequired << ")\n";
<< getSyclObjImpl(Device).get() << ")\n";

std::cerr << "available device images:\n";
debugPrintBinaryImages();
Expand Down Expand Up @@ -1569,8 +1544,6 @@ RTDeviceBinaryImage &ProgramManager::getDeviceImage(
ImageIterator = ImageSet.begin();
std::advance(ImageIterator, ImgInd);

CheckJITCompilationForImage(*ImageIterator, JITCompilationIsRequired);

if constexpr (DbgProgMgr > 0) {
std::cerr << "selected device image: " << &(*ImageIterator)->getRawData()
<< "\n";
Expand Down
11 changes: 3 additions & 8 deletions sycl/source/detail/program_manager/program_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,11 @@ class ProgramManager {

RTDeviceBinaryImage &getDeviceImage(const std::string &KernelName,
const context &Context,
const device &Device,
bool JITCompilationIsRequired = false);
const device &Device);

RTDeviceBinaryImage &getDeviceImage(
const std::unordered_set<RTDeviceBinaryImage *> &ImagesToVerify,
const context &Context, const device &Device,
bool JITCompilationIsRequired = false);
const context &Context, const device &Device);

ur_program_handle_t createURProgram(const RTDeviceBinaryImage &Img,
const context &Context,
Expand Down Expand Up @@ -177,13 +175,10 @@ class ProgramManager {
/// \param Context the context to build the program with
/// \param Device the device for which the program is built
/// \param KernelName the kernel's name
/// \param JITCompilationIsRequired If JITCompilationIsRequired is true
/// add a check that kernel is compiled, otherwise don't add the check.
ur_program_handle_t getBuiltURProgram(const ContextImplPtr &ContextImpl,
const DeviceImplPtr &DeviceImpl,
const std::string &KernelName,
const NDRDescT &NDRDesc = {},
bool JITCompilationIsRequired = false);
const NDRDescT &NDRDesc = {});

/// Builds a program from a given set of images or retrieves that program from
/// cache.
Expand Down

0 comments on commit cd30a59

Please sign in to comment.