Skip to content

[SYCL] Change NativePrograms.insert to [] access #14873

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
merged 1 commit into from
Aug 2, 2024
Merged

[SYCL] Change NativePrograms.insert to [] access #14873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions sycl/source/detail/program_manager/program_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ ProgramManager::createURProgram(const RTDeviceBinaryImage &Img,
{
std::lock_guard<std::mutex> Lock(MNativeProgramsMutex);
// associate the UR program with the image it was created for
NativePrograms.insert({Res, &Img});
NativePrograms[Res] = &Img;
}

Ctx->addDeviceGlobalInitializer(Res, {Device}, &Img);
Expand Down Expand Up @@ -840,9 +840,9 @@ ur_program_handle_t ProgramManager::getBuiltURProgram(

{
std::lock_guard<std::mutex> Lock(MNativeProgramsMutex);
NativePrograms.insert({BuiltProgram.get(), &Img});
NativePrograms[BuiltProgram.get()] = &Img;
for (RTDeviceBinaryImage *LinkedImg : DeviceImagesToLink) {
NativePrograms.insert({BuiltProgram.get(), LinkedImg});
NativePrograms[BuiltProgram.get()] = LinkedImg;
}
}

Expand Down Expand Up @@ -2500,7 +2500,7 @@ device_image_plain ProgramManager::build(const device_image_plain &DeviceImage,

{
std::lock_guard<std::mutex> Lock(MNativeProgramsMutex);
NativePrograms.insert({BuiltProgram.get(), &Img});
NativePrograms[BuiltProgram.get()] = &Img;
}

ContextImpl->addDeviceGlobalInitializer(BuiltProgram.get(), Devs, &Img);
Expand Down
103 changes: 103 additions & 0 deletions sycl/unittests/program_manager/arg_mask/EliminatedArgMask.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@

class EAMTestKernel;
class EAMTestKernel2;
class EAMTestKernel3;
constexpr const char EAMTestKernelName[] = "EAMTestKernel";
constexpr const char EAMTestKernel2Name[] = "EAMTestKernel2";
constexpr const char EAMTestKernel3Name[] = "EAMTestKernel3";
constexpr unsigned EAMTestKernelNumArgs = 4;

namespace sycl {
Expand All @@ -39,6 +41,12 @@ struct KernelInfo<EAMTestKernel2> : public unittest::MockKernelInfoBase {
static constexpr const char *getName() { return EAMTestKernel2Name; }
};

template <>
struct KernelInfo<EAMTestKernel3> : public unittest::MockKernelInfoBase {
static constexpr unsigned getNumParams() { return EAMTestKernelNumArgs; }
static constexpr const char *getName() { return EAMTestKernel3Name; }
};

} // namespace detail
} // namespace _V1
} // namespace sycl
Expand Down Expand Up @@ -90,10 +98,39 @@ static sycl::unittest::UrImage generateEAMTestKernel2Image() {
return Img;
}

static sycl::unittest::UrImage generateEAMTestKernel3Image() {
using namespace sycl::unittest;

// Eliminated arguments are 2nd and 4th.
std::vector<unsigned char> KernelEAM{0b00001010};
UrProperty EAMKernelPOI = makeKernelParamOptInfo(
EAMTestKernel3Name, EAMTestKernelNumArgs, KernelEAM);
UrArray<UrProperty> ImgKPOI{std::move(EAMKernelPOI)};

UrPropertySet PropSet;
PropSet.insert(__SYCL_PROPERTY_SET_KERNEL_PARAM_OPT_INFO, std::move(ImgKPOI));

std::vector<unsigned char> Bin{0, 1, 2, 3, 4, 5}; // Random data

UrArray<UrOffloadEntry> Entries = makeEmptyKernels({EAMTestKernel3Name});

UrImage Img{SYCL_DEVICE_BINARY_TYPE_SPIRV, // Format
__SYCL_DEVICE_BINARY_TARGET_SPIRV64, // DeviceTargetSpec
"", // Compile options
"", // Link options
std::move(Bin),
std::move(Entries),
std::move(PropSet)};

return Img;
}

static sycl::unittest::UrImage EAMImg = generateEAMTestKernelImage();
static sycl::unittest::UrImage EAM2Img = generateEAMTestKernel2Image();
static sycl::unittest::UrImage EAM3Img = generateEAMTestKernel3Image();
static sycl::unittest::UrImageArray<1> EAMImgArray{&EAMImg};
static sycl::unittest::UrImageArray<1> EAM2ImgArray{&EAM2Img};
static sycl::unittest::UrImageArray<1> EAM3ImgArray{&EAM3Img};

// ur_program_handle_t address is used as a key for ProgramManager::NativePrograms
// storage. redefinedProgramLinkCommon makes ur_program_handle_t address equal to 0x1.
Expand All @@ -106,6 +143,17 @@ inline ur_result_t redefinedProgramCreateEAM(void *pParams) {
return UR_RESULT_SUCCESS;
}

mock::dummy_handle_t_ FixedHandle;
inline ur_result_t setFixedProgramPtr(void *pParams) {
auto params = *static_cast<ur_program_create_with_il_params_t *>(pParams);
**params.pphProgram = reinterpret_cast<ur_program_handle_t>(&FixedHandle);
return UR_RESULT_SUCCESS;
}
inline ur_result_t releaseFixedProgramPtr(void *pParams) {
// Do nothing
return UR_RESULT_SUCCESS;
}

class MockHandler : public sycl::handler {

public:
Expand Down Expand Up @@ -203,3 +251,58 @@ TEST(EliminatedArgMask, KernelBundleWith2Kernels) {

EXPECT_EQ(*EliminatedArgMask, ExpElimArgMask);
}

// It's possible for the same handle to be reused for multiple distinct programs
// This can happen if a program is released (freeing underlying memory) and then
// a new program happens to get given that same memory for its handle.
// The ProgramContext stores a map with `ur_program_handle_t`s, which are never
// cleared. This test ensures that newer `ur_program_handle_t`s with the same
// values override older ones.
TEST(EliminatedArgMask, ReuseOfHandleValues) {
sycl::detail::ProgramManager &PM =
sycl::detail::ProgramManager::getInstance();

ur_program_handle_t ProgBefore = nullptr;
ur_program_handle_t ProgAfter = nullptr;
{
auto Name = sycl::detail::KernelInfo<EAMTestKernel>::getName();
sycl::unittest::UrMock<> Mock;
sycl::platform Plt = sycl::platform();
mock::getCallbacks().set_replace_callback("urProgramCreateWithIL",
&setFixedProgramPtr);
mock::getCallbacks().set_replace_callback("urProgramRelease",
&releaseFixedProgramPtr);

const sycl::device Dev = Plt.get_devices()[0];
sycl::queue Queue{Dev};
auto Ctx = Queue.get_context();
ProgBefore = PM.getBuiltURProgram(sycl::detail::getSyclObjImpl(Ctx),
sycl::detail::getSyclObjImpl(Dev), Name);
auto Mask = PM.getEliminatedKernelArgMask(ProgBefore, Name);
EXPECT_NE(Mask, nullptr);
EXPECT_EQ(Mask->at(0), 1);
}

{
auto Name = sycl::detail::KernelInfo<EAMTestKernel3>::getName();
sycl::unittest::UrMock<> Mock;
sycl::platform Plt = sycl::platform();
mock::getCallbacks().set_replace_callback("urProgramCreateWithIL",
&setFixedProgramPtr);
mock::getCallbacks().set_replace_callback("urProgramRelease",
&releaseFixedProgramPtr);

const sycl::device Dev = Plt.get_devices()[0];
sycl::queue Queue{Dev};
auto Ctx = Queue.get_context();
ProgAfter = PM.getBuiltURProgram(sycl::detail::getSyclObjImpl(Ctx),
sycl::detail::getSyclObjImpl(Dev), Name);
auto Mask = PM.getEliminatedKernelArgMask(ProgAfter, Name);
EXPECT_NE(Mask, nullptr);
EXPECT_EQ(Mask->at(0), 0);
}

// Verify that the test is behaving correctly and that the pointer is being
// reused
EXPECT_EQ(ProgBefore, ProgAfter);
}
Loading