Skip to content

Commit b1b4e68

Browse files
[SYCL][ESIMD][EMU] Improve buffer/image memory management (#5930)
- introduce cm_surface_ptr_t to represent a 'universal' pointer to either sycl buffer or image along with necessary info - move surface allocation/deallocation to _pi_mem ctor/dtor from PI interface implementations for better logic encapsulation.
1 parent a258a55 commit b1b4e68

File tree

2 files changed

+76
-136
lines changed

2 files changed

+76
-136
lines changed

sycl/plugins/esimd_emulator/pi_esimd_emulator.cpp

Lines changed: 41 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,17 +1011,17 @@ pi_result piMemBufferCreate(pi_context Context, pi_mem_flags Flags, size_t Size,
10111011
}
10121012

10131013
char *MapBasePtr = nullptr;
1014-
cm_buffer_ptr_slot CmBuf;
1014+
cm_surface_ptr_t CmBuf;
10151015
cm_support::SurfaceIndex *CmIndex;
10161016
int Status = cm_support::CM_FAILURE;
10171017

10181018
if (Flags & PI_MEM_FLAGS_HOST_PTR_USE) {
1019-
CmBuf.tag = cm_buffer_ptr_slot::type_user_provided;
1019+
CmBuf.tag = cm_surface_ptr_t::TypeUserProvidedBuffer;
10201020
Status = Context->Device->CmDevicePtr->CreateBufferUP(
10211021
static_cast<unsigned int>(Size), HostPtr, CmBuf.UPBufPtr);
10221022
CmBuf.UPBufPtr->GetIndex(CmIndex);
10231023
} else {
1024-
CmBuf.tag = cm_buffer_ptr_slot::type_regular;
1024+
CmBuf.tag = cm_surface_ptr_t::TypeRegularBuffer;
10251025
Status = Context->Device->CmDevicePtr->CreateBuffer(
10261026
static_cast<unsigned int>(Size), CmBuf.RegularBufPtr);
10271027
CmBuf.RegularBufPtr->GetIndex(CmIndex);
@@ -1077,58 +1077,40 @@ pi_result piMemRelease(pi_mem Mem) {
10771077
}
10781078

10791079
if (--(Mem->RefCount) == 0) {
1080-
int Status = cm_support::CM_FAILURE;
1081-
1082-
if (Mem->getMemType() == PI_MEM_TYPE_BUFFER) {
1083-
_pi_buffer *PiBuf = static_cast<_pi_buffer *>(Mem);
1084-
if (PiBuf->BufferPtr.tag == cm_buffer_ptr_slot::type_user_provided) {
1085-
Status = Mem->Context->Device->CmDevicePtr->DestroyBufferUP(
1086-
PiBuf->BufferPtr.UPBufPtr);
1087-
} else {
1088-
Status = Mem->Context->Device->CmDevicePtr->DestroySurface(
1089-
PiBuf->BufferPtr.RegularBufPtr);
1090-
}
1091-
} else if (Mem->getMemType() == PI_MEM_TYPE_IMAGE2D) {
1092-
_pi_image *PiImg = static_cast<_pi_image *>(Mem);
1093-
if (PiImg->ImagePtr.tag == cm_image_ptr_slot::type_user_provided) {
1094-
Status = Mem->Context->Device->CmDevicePtr->DestroySurface2DUP(
1095-
PiImg->ImagePtr.UPImgPtr);
1096-
} else {
1097-
Status = Mem->Context->Device->CmDevicePtr->DestroySurface(
1098-
PiImg->ImagePtr.RegularImgPtr);
1099-
}
1100-
}
1101-
1102-
if (Status != cm_support::CM_SUCCESS) {
1103-
return PI_INVALID_MEM_OBJECT;
1104-
}
1105-
11061080
// Removing Surface-map entry
11071081
std::lock_guard<std::mutex> Lock{*PiESimdSurfaceMapLock};
11081082
auto MapEntryIt = PiESimdSurfaceMap->find(Mem->SurfaceIndex);
1109-
if (MapEntryIt != PiESimdSurfaceMap->end()) {
1110-
PiESimdSurfaceMap->erase(MapEntryIt);
1111-
} else {
1112-
if (PrintPiTrace) {
1113-
std::cerr << "Failure from CM-managed buffer/image deletion"
1114-
<< std::endl;
1115-
}
1116-
return PI_INVALID_MEM_OBJECT;
1117-
}
1118-
1119-
// TODO : Erasing should be done during 'piMemRelease'? Or Host has
1120-
// to call 'piEnqueueMemUnmap' for all mapped addresses before
1121-
// calling 'piMemRelease'?
1122-
std::lock_guard<std::mutex> MapLock{Mem->MappingsMutex};
1123-
for (auto mapit = Mem->Mappings.begin(); mapit != Mem->Mappings.end();) {
1124-
mapit = Mem->Mappings.erase(mapit);
1125-
}
1126-
1083+
assert(MapEntryIt != PiESimdSurfaceMap->end() &&
1084+
"Failure from Buffer/Image deletion");
1085+
PiESimdSurfaceMap->erase(MapEntryIt);
11271086
delete Mem;
11281087
}
11291088
return PI_SUCCESS;
11301089
}
11311090

1091+
_pi_mem::~_pi_mem() {
1092+
int Status = cm_support::CM_FAILURE;
1093+
1094+
cm_support::CmDevice *CmDevice = Context->Device->CmDevicePtr;
1095+
1096+
if (SurfacePtr.tag == cm_surface_ptr_t::TypeUserProvidedBuffer) {
1097+
Status = CmDevice->DestroyBufferUP(SurfacePtr.UPBufPtr);
1098+
} else if (SurfacePtr.tag == cm_surface_ptr_t::TypeRegularBuffer) {
1099+
Status = CmDevice->DestroySurface(SurfacePtr.RegularBufPtr);
1100+
} else if (SurfacePtr.tag == cm_surface_ptr_t::TypeUserProvidedImage) {
1101+
Status = CmDevice->DestroySurface2DUP(SurfacePtr.UPImgPtr);
1102+
} else if (SurfacePtr.tag == cm_surface_ptr_t::TypeRegularImage) {
1103+
Status = CmDevice->DestroySurface(SurfacePtr.RegularImgPtr);
1104+
}
1105+
1106+
assert(Status == cm_support::CM_SUCCESS &&
1107+
"Surface Deletion Failure from CM_EMU");
1108+
1109+
for (auto mapit = Mappings.begin(); mapit != Mappings.end();) {
1110+
mapit = Mappings.erase(mapit);
1111+
}
1112+
}
1113+
11321114
cm_support::CM_SURFACE_FORMAT
11331115
ConvertPiImageFormatToCmFormat(const pi_image_format *PiFormat) {
11341116
using ULongPair = std::pair<unsigned long, unsigned long>;
@@ -1219,18 +1201,19 @@ pi_result piMemImageCreate(pi_context Context, pi_mem_flags Flags,
12191201
}
12201202

12211203
char *MapBasePtr = nullptr;
1222-
cm_image_ptr_slot CmImg;
1204+
cm_surface_ptr_t CmImg;
12231205
cm_support::SurfaceIndex *CmIndex;
12241206
int Status = cm_support::CM_SUCCESS;
12251207

12261208
if (Flags & PI_MEM_FLAGS_HOST_PTR_USE) {
1227-
CmImg.tag = cm_image_ptr_slot::type_user_provided;
1209+
CmImg.tag = cm_surface_ptr_t::TypeUserProvidedImage;
12281210
Status = Context->Device->CmDevicePtr->CreateSurface2DUP(
12291211
static_cast<unsigned int>(ImageDesc->image_width),
12301212
static_cast<unsigned int>(ImageDesc->image_height), CmSurfFormat,
12311213
HostPtr, CmImg.UPImgPtr);
12321214
CmImg.UPImgPtr->GetIndex(CmIndex);
12331215
} else {
1216+
CmImg.tag = cm_surface_ptr_t::TypeRegularImage;
12341217
Status = Context->Device->CmDevicePtr->CreateSurface2D(
12351218
static_cast<unsigned int>(ImageDesc->image_width),
12361219
static_cast<unsigned int>(ImageDesc->image_height), CmSurfFormat,
@@ -1520,12 +1503,13 @@ pi_result piEnqueueMemBufferRead(pi_queue Queue, pi_mem Src,
15201503
RetEv->IsDummyEvent = true;
15211504
}
15221505

1523-
if (buf->BufferPtr.tag == cm_buffer_ptr_slot::type_user_provided) {
1506+
if (buf->SurfacePtr.tag == cm_surface_ptr_t::TypeUserProvidedBuffer) {
15241507
// CM does not provide 'ReadSurface' call for 'User-Provided'
15251508
// Surface. memcpy is used for BufferRead PI_API call.
15261509
memcpy(Dst, buf->MapHostPtr, Size);
15271510
} else {
1528-
int Status = buf->BufferPtr.RegularBufPtr->ReadSurface(
1511+
assert(buf->SurfacePtr.tag == cm_surface_ptr_t::TypeRegularBuffer);
1512+
int Status = buf->SurfacePtr.RegularBufPtr->ReadSurface(
15291513
reinterpret_cast<unsigned char *>(Dst),
15301514
nullptr, // event
15311515
static_cast<uint64_t>(Size));
@@ -1706,12 +1690,13 @@ pi_result piEnqueueMemImageRead(pi_queue CommandQueue, pi_mem Image,
17061690
}
17071691

17081692
size_t Size = RowPitch * (Region->height);
1709-
if (PiImg->ImagePtr.tag == cm_image_ptr_slot::type_user_provided) {
1693+
if (PiImg->SurfacePtr.tag == cm_surface_ptr_t::TypeUserProvidedImage) {
17101694
// CM does not provide 'ReadSurface' call for 'User-Provided'
17111695
// Surface. memcpy is used for ImageRead PI_API call.
17121696
memcpy(Ptr, PiImg->MapHostPtr, Size);
17131697
} else {
1714-
int Status = PiImg->ImagePtr.RegularImgPtr->ReadSurface(
1698+
assert(PiImg->SurfacePtr.tag == cm_surface_ptr_t::TypeRegularImage);
1699+
int Status = PiImg->SurfacePtr.RegularImgPtr->ReadSurface(
17151700
reinterpret_cast<unsigned char *>(Ptr),
17161701
nullptr, // event
17171702
static_cast<uint64_t>(Size));
@@ -1974,43 +1959,9 @@ pi_result piTearDown(void *) {
19741959

19751960
for (auto it = PiESimdSurfaceMap->begin(); it != PiESimdSurfaceMap->end();) {
19761961
auto Mem = it->second;
1977-
if (Mem == nullptr) {
1978-
/// Skipping map entry for SLM_BTI
1979-
it = PiESimdSurfaceMap->erase(it);
1980-
continue;
1981-
}
1982-
int Status = cm_support::CM_FAILURE;
1983-
1984-
if (Mem->getMemType() == PI_MEM_TYPE_BUFFER) {
1985-
_pi_buffer *PiBuf = static_cast<_pi_buffer *>(Mem);
1986-
if (PiBuf->BufferPtr.tag == cm_buffer_ptr_slot::type_user_provided) {
1987-
Status = Mem->Context->Device->CmDevicePtr->DestroyBufferUP(
1988-
PiBuf->BufferPtr.UPBufPtr);
1989-
} else {
1990-
Status = Mem->Context->Device->CmDevicePtr->DestroySurface(
1991-
PiBuf->BufferPtr.RegularBufPtr);
1992-
}
1993-
} else if (Mem->getMemType() == PI_MEM_TYPE_IMAGE2D) {
1994-
_pi_image *PiImg = static_cast<_pi_image *>(Mem);
1995-
if (PiImg->ImagePtr.tag == cm_image_ptr_slot::type_user_provided) {
1996-
Status = Mem->Context->Device->CmDevicePtr->DestroySurface2DUP(
1997-
PiImg->ImagePtr.UPImgPtr);
1998-
} else {
1999-
Status = Mem->Context->Device->CmDevicePtr->DestroySurface(
2000-
PiImg->ImagePtr.RegularImgPtr);
2001-
}
2002-
}
2003-
2004-
if (Status != cm_support::CM_SUCCESS) {
2005-
return PI_INVALID_MEM_OBJECT;
2006-
}
2007-
2008-
// No "MappingsMutex" as piTearDown is guaranteed to be called
2009-
// from single thread for plug-in
2010-
for (auto mapit = Mem->Mappings.begin(); mapit != Mem->Mappings.end();) {
2011-
mapit = Mem->Mappings.erase(mapit);
2012-
}
2013-
1962+
if (Mem != nullptr) {
1963+
delete Mem;
1964+
} // else { /* Null-entry for SLM_BTI */ }
20141965
it = PiESimdSurfaceMap->erase(it);
20151966
}
20161967
return PI_SUCCESS;

sycl/plugins/esimd_emulator/pi_esimd_emulator.hpp

Lines changed: 35 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ struct _pi_object {
5252
_pi_object() : RefCount{1} {}
5353

5454
std::atomic<pi_uint32> RefCount;
55+
56+
virtual ~_pi_object() = default;
5557
};
5658
struct _pi_platform {
5759
_pi_platform() = default;
@@ -105,9 +107,28 @@ struct _pi_queue : _pi_object {
105107
cm_support::CmQueue *CmQueuePtr = nullptr;
106108
};
107109

108-
struct _pi_mem : _pi_object {
109-
_pi_mem() = default;
110+
struct cm_surface_ptr_t {
111+
// 'UP' means 'User-Provided' in CM Lib - corresponding to
112+
// Buffer/Image created with PI_MEM_FLAGS_HOST_PTR_USE option in
113+
// SYCL
114+
enum SurfaceType {
115+
TypeNone,
116+
TypeRegularBuffer,
117+
TypeUserProvidedBuffer,
118+
TypeRegularImage,
119+
TypeUserProvidedImage
120+
};
121+
SurfaceType tag = TypeNone;
122+
123+
union {
124+
cm_support::CmBuffer *RegularBufPtr = nullptr;
125+
cm_support::CmBufferUP *UPBufPtr;
126+
cm_support::CmSurface2D *RegularImgPtr;
127+
cm_support::CmSurface2DUP *UPImgPtr;
128+
};
129+
};
110130

131+
struct _pi_mem : _pi_object {
111132
pi_context Context;
112133

113134
// To be used for piEnqueueMemBufferMap
@@ -135,67 +156,35 @@ struct _pi_mem : _pi_object {
135156
// Supporing multi-threaded mapping/unmapping calls
136157
std::mutex MappingsMutex;
137158

138-
_pi_mem_type getMemType() const { return MemType; };
159+
cm_surface_ptr_t SurfacePtr;
160+
161+
// Destructor for invoking buffer/image destory calls in CM Runtime
162+
~_pi_mem();
139163

140164
protected:
141-
_pi_mem(pi_context ctxt, char *HostPtr, _pi_mem_type MemTypeArg,
165+
_pi_mem(pi_context ctxt, char *HostPtr, cm_surface_ptr_t SurfacePtrArg,
142166
unsigned int SurfaceIdxArg)
143-
: Context{ctxt}, MapHostPtr{HostPtr},
144-
SurfaceIndex{SurfaceIdxArg}, MemType{MemTypeArg} {}
145-
146-
private:
147-
_pi_mem_type MemType;
148-
};
149-
150-
// TODO: Merge cm_buffer_ptr_slot and cm_image_ptr_slot into one
151-
// struct
152-
struct cm_buffer_ptr_slot {
153-
// 'UP' means 'User-Provided' in CM Lib - corresponding to
154-
// Host-created buffer in SYCL
155-
156-
enum type { type_none, type_regular, type_user_provided };
157-
type tag = type_none;
158-
159-
union {
160-
cm_support::CmBuffer *RegularBufPtr = nullptr;
161-
cm_support::CmBufferUP *UPBufPtr;
162-
};
163-
};
164-
165-
struct cm_image_ptr_slot {
166-
// 'UP' means 'User-Provided' in CM Lib - corresponding to
167-
// Host-created image in SYCL
168-
169-
enum type { type_none, type_regular, type_user_provided };
170-
type tag = type_none;
171-
172-
union {
173-
cm_support::CmSurface2D *RegularImgPtr = nullptr;
174-
cm_support::CmSurface2DUP *UPImgPtr;
175-
};
167+
: Context{ctxt}, MapHostPtr{HostPtr}, SurfaceIndex{SurfaceIdxArg},
168+
SurfacePtr{SurfacePtrArg} {}
176169
};
177170

178171
struct _pi_buffer final : _pi_mem {
179172
// Buffer/Sub-buffer constructor
180-
_pi_buffer(pi_context ctxt, char *HostPtr, cm_buffer_ptr_slot BufferPtrArg,
173+
_pi_buffer(pi_context ctxt, char *HostPtr, cm_surface_ptr_t SurfacePtrArg,
181174
unsigned int SurfaceIdxArg, size_t SizeArg)
182-
: _pi_mem(ctxt, HostPtr, PI_MEM_TYPE_BUFFER, SurfaceIdxArg),
183-
BufferPtr{BufferPtrArg}, Size{SizeArg} {}
175+
: _pi_mem(ctxt, HostPtr, SurfacePtrArg, SurfaceIdxArg), Size{SizeArg} {}
184176

185-
cm_buffer_ptr_slot BufferPtr;
186177
size_t Size;
187178
};
188179

189180
struct _pi_image final : _pi_mem {
190181
// Image constructor
191-
_pi_image(pi_context ctxt, char *HostPtr, cm_image_ptr_slot ImagePtrArg,
182+
_pi_image(pi_context ctxt, char *HostPtr, cm_surface_ptr_t SurfacePtrArg,
192183
unsigned int SurfaceIdxArg, size_t WidthArg, size_t HeightArg,
193184
size_t BPPArg)
194-
: _pi_mem(ctxt, HostPtr, PI_MEM_TYPE_IMAGE2D, SurfaceIdxArg),
195-
ImagePtr(ImagePtrArg), Width{WidthArg}, Height{HeightArg},
196-
BytesPerPixel{BPPArg} {}
185+
: _pi_mem(ctxt, HostPtr, SurfacePtrArg, SurfaceIdxArg), Width{WidthArg},
186+
Height{HeightArg}, BytesPerPixel{BPPArg} {}
197187

198-
cm_image_ptr_slot ImagePtr;
199188
size_t Width;
200189
size_t Height;
201190
size_t BytesPerPixel;

0 commit comments

Comments
 (0)