[SYCL][ESIMD] RT: remove wrapping buffer objects into images.

BE is now able to accept buffers instead of image1d buffers, so remove the
w/a wrapping. This patch also gets rid of majority of __SYCL_EXPLICIT_SIMD__
macro checks in the API code, paving a way for ESIMD/SYCL kernel
co-existence.

Details:
- Use new int header feature KernelInfo<>::isESIMD() to properly set
  accessor args
- Add __init_esimd accessor methods to be used by FE with ESIMD kernels
- Some ESIMD-related stuff is now unsed - can't be removed now not to break
  ABI. TODOs added.

Signed-off-by: Konstantin S Bobrovsky <[email protected]>

Author: kbobrovs

sycl/include/CL/sycl/accessor.hpp

@@ -859,6 +859,9 @@ class accessor :
   detail::AccessorImplDevice<AdjustedDim> impl;
 
 #ifdef __SYCL_EXPLICIT_SIMD__
+  // TODO all the Image1dBuffer* stuff, including the union with MData field
+  // below is not used anymore and is left temporarily to avoid ABI breaking
+  // changes.
   using OCLImage1dBufferTy =
       typename detail::opencl_image1d_buffer_type<AccessMode>::type;
 #endif // __SYCL_EXPLICIT_SIMD__
@@ -870,15 +873,9 @@ class accessor :
 #endif // __SYCL_EXPLICIT_SIMD__
   };
 
-#ifdef __SYCL_EXPLICIT_SIMD__
-  // TODO In ESIMD accessors usage is limited for now - access range, mem
-  // range and offset are not supported. The cl_mem object allocated for
-  // a global accessor is always wrapped into a 1d image buffer to enable
-  // surface index-based addressing.
-  void __init(OCLImage1dBufferTy ImgBuf) { ImageBuffer = ImgBuf; }
+  // TODO replace usages with getQualifiedPtr
+  const ConcreteASPtrType getNativeImageObj() const { return MData; }
 
-  const OCLImage1dBufferTy getNativeImageObj() const { return ImageBuffer; }
-#else
   void __init(ConcreteASPtrType Ptr, range<AdjustedDim> AccessRange,
               range<AdjustedDim> MemRange, id<AdjustedDim> Offset) {
     MData = Ptr;
@@ -893,7 +890,12 @@ class accessor :
     if (1 == AdjustedDim)
       MData += Offset[0];
   }
-#endif // __SYCL_EXPLICIT_SIMD__
+
+  // __init variant used by the device compiler for ESIMD kernels.
+  // TODO In ESIMD accessors usage is limited for now - access range, mem
+  // range and offset are not supported.
+  void __init_esimd(ConcreteASPtrType Ptr) { MData = Ptr; }
+
   ConcreteASPtrType getQualifiedPtr() const { return MData; }
 
   template <typename, int, access::mode, access::target, access::placeholder,
@@ -1963,6 +1965,9 @@ class accessor<DataT, Dimensions, AccessMode, access::target::image,
   // It does not call the base class's init method.
   void __init(OCLImageTy Image) { this->imageAccessorInit(Image); }
 
+  // __init variant used by the device compiler for ESIMD kernels.
+  void __init_esimd(OCLImageTy Image) { this->imageAccessorInit(Image); }
+
 public:
   // Default constructor for objects later initialized with __init member.
   accessor() = default;
@@ -2014,6 +2019,9 @@ class accessor<DataT, Dimensions, AccessMode, access::target::image_array,
   // It does not call the base class's init method.
   void __init(OCLImageTy Image) { this->imageAccessorInit(Image); }
 
+  // __init variant used by the device compiler for ESIMD kernels.
+  void __init_esimd(OCLImageTy Image) { this->imageAccessorInit(Image); }
+
 public:
   // Default constructor for objects later initialized with __init member.
   accessor() = default;

sycl/include/CL/sycl/detail/accessor_impl.hpp

@@ -70,29 +70,16 @@ template <int Dims> class LocalAccessorBaseDevice {
   }
 };
 
-// TODO ESIMD Currently all accessors are treated as ESIMD under corresponding
-// compiler option enabling the macro below. Eventually ESIMD kernels and usual
-// kernels must co-exist and there must be a mechanism for distinguishing usual
-// and ESIMD accessors.
-#ifndef __SYCL_EXPLICIT_SIMD__
-constexpr bool IsESIMDAccInit = false;
-#else
-constexpr bool IsESIMDAccInit = true;
-#endif // __SYCL_EXPLICIT_SIMD__
-
 class __SYCL_EXPORT AccessorImplHost {
 public:
   AccessorImplHost(id<3> Offset, range<3> AccessRange, range<3> MemoryRange,
                    access::mode AccessMode, detail::SYCLMemObjI *SYCLMemObject,
                    int Dims, int ElemSize, int OffsetInBytes = 0,
-                   bool IsSubBuffer = false, bool IsESIMDAcc = IsESIMDAccInit)
+                   bool IsSubBuffer = false)
       : MOffset(Offset), MAccessRange(AccessRange), MMemoryRange(MemoryRange),
         MAccessMode(AccessMode), MSYCLMemObj(SYCLMemObject), MDims(Dims),
         MElemSize(ElemSize), MOffsetInBytes(OffsetInBytes),
-        MIsSubBuffer(IsSubBuffer) {
-    MIsESIMDAcc =
-        IsESIMDAcc && (SYCLMemObject->getType() == SYCLMemObjI::BUFFER);
-  }
+        MIsSubBuffer(IsSubBuffer) {}
 
   ~AccessorImplHost();
 
@@ -101,7 +88,7 @@ class __SYCL_EXPORT AccessorImplHost {
         MMemoryRange(Other.MMemoryRange), MAccessMode(Other.MAccessMode),
         MSYCLMemObj(Other.MSYCLMemObj), MDims(Other.MDims),
         MElemSize(Other.MElemSize), MOffsetInBytes(Other.MOffsetInBytes),
-        MIsSubBuffer(Other.MIsSubBuffer), MIsESIMDAcc(Other.MIsESIMDAcc) {}
+        MIsSubBuffer(Other.MIsSubBuffer) {}
 
   // The resize method provides a way to change the size of the
   // allocated memory and corresponding properties for the accessor.
@@ -133,9 +120,6 @@ class __SYCL_EXPORT AccessorImplHost {
   Command *MBlockedCmd = nullptr;
 
   bool PerWI = false;
-
-  // Whether this accessor is ESIMD accessor with special memory allocation.
-  bool MIsESIMDAcc;
 };
 
 using AccessorImplPtr = shared_ptr_class<AccessorImplHost>;
@@ -148,8 +132,7 @@ class AccessorBaseHost {
                    bool IsSubBuffer = false) {
     impl = shared_ptr_class<AccessorImplHost>(new AccessorImplHost(
         Offset, AccessRange, MemoryRange, AccessMode, SYCLMemObject, Dims,
-        ElemSize, OffsetInBytes, IsSubBuffer,
-        IsESIMDAccInit && (SYCLMemObject->getType() == SYCLMemObjI::BUFFER)));
+        ElemSize, OffsetInBytes, IsSubBuffer));
   }
 
 protected:

sycl/include/CL/sycl/detail/image_ocl_types.hpp

@@ -193,6 +193,9 @@ inline int getSPIRVElementSize(int ImageChannelType, int ImageChannelOrder) {
 }
 
 #ifdef __SYCL_EXPLICIT_SIMD__
+// TODO all the opencl_image1d_buffer* stuff below is not used anymore and is
+// left temporarily to avoid ABI breaking changes - field of this type is
+// temporarily present in the accessor class.
 template <access::mode AccessMode> struct opencl_image1d_buffer_type;
 
 // OpenCL types used only when compiling DPCPP ESIMD kernels

sycl/include/CL/sycl/detail/kernel_desc.hpp

@@ -56,6 +56,7 @@ template <class KernelNameType> struct KernelInfo {
     return Dummy;
   }
   static constexpr const char *getName() { return ""; }
+  static constexpr bool isESIMD() { return 0; }
 };
 #else
 template <char...> struct KernelInfoData {
@@ -65,6 +66,7 @@ template <char...> struct KernelInfoData {
     return Dummy;
   }
   static constexpr const char *getName() { return ""; }
+  static constexpr bool isESIMD() { return 0; }
 };
 
 // C++14 like index_sequence and make_index_sequence

sycl/include/CL/sycl/detail/memory_manager.hpp

@@ -49,11 +49,13 @@ class __SYCL_EXPORT MemoryManager {
                         RT::PiEvent &OutEvent);
 
   // Allocates memory buffer wrapped into an image. MemObj must be a buffer,
-  // not an image. Used in ESIMD extension to enable surface index-based access.
+  // not an image.
+  // TODO not used - remove.
   static void *wrapIntoImageBuffer(ContextImplPtr TargetContext, void *MemBuf,
                                    SYCLMemObjI *MemObj);
 
   // Releases the image buffer created by wrapIntoImageBuffer.
+  // TODO not used - remove.
   static void releaseImageBuffer(ContextImplPtr TargetContext, void *ImageBuf);
 
   // The following method creates OpenCL sub buffer for specified

sycl/include/CL/sycl/handler.hpp

@@ -318,17 +318,30 @@ class __SYCL_EXPORT handler {
 
   /// Extracts and prepares kernel arguments from the lambda using integration
   /// header.
+  /// TODO replace with the version below once ABI breaking changes are allowed.
   void
   extractArgsAndReqsFromLambda(char *LambdaPtr, size_t KernelArgsNum,
                                const detail::kernel_param_desc_t *KernelArgs);
 
+  /// Extracts and prepares kernel arguments from the lambda using integration
+  /// header.
+  void
+  extractArgsAndReqsFromLambda(char *LambdaPtr, size_t KernelArgsNum,
+                               const detail::kernel_param_desc_t *KernelArgs,
+                               bool IsESIMD);
+
   /// Extracts and prepares kernel arguments set via set_arg(s).
   void extractArgsAndReqs();
 
+  /// TODO replace with the version below once ABI breaking changes are allowed.
   void processArg(void *Ptr, const detail::kernel_param_kind_t &Kind,
                   const int Size, const size_t Index, size_t &IndexShift,
                   bool IsKernelCreatedFromSource);
 
+  void processArg(void *Ptr, const detail::kernel_param_kind_t &Kind,
+                  const int Size, const size_t Index, size_t &IndexShift,
+                  bool IsKernelCreatedFromSource, bool IsESIMD);
+
   /// \return a string containing name of SYCL kernel.
   string_class getKernelName();
 
@@ -490,9 +503,10 @@ class __SYCL_EXPORT handler {
     // Empty name indicates that the compilation happens without integration
     // header, so don't perform things that require it.
     if (KI::getName() != nullptr && KI::getName()[0] != '\0') {
+      // TODO support ESIMD in no-integration-header case too.
       MArgs.clear();
       extractArgsAndReqsFromLambda(MHostKernel->getPtr(), KI::getNumParams(),
-                                   &KI::getParamDesc(0));
+                                   &KI::getParamDesc(0), KI::isESIMD());
       MKernelName = KI::getName();
       MOSModuleHandle = detail::OSUtil::getOSModuleHandle(KI::getName());
     } else {

sycl/source/detail/memory_manager.cpp

@@ -47,8 +47,8 @@ void MemoryManager::release(ContextImplPtr TargetContext, SYCLMemObjI *MemObj,
 
 void MemoryManager::releaseImageBuffer(ContextImplPtr TargetContext,
                                        void *ImageBuf) {
-  auto PIObj = reinterpret_cast<pi_mem>(ImageBuf);
-  TargetContext->getPlugin().call<PiApiKind::piMemRelease>(PIObj);
+  // TODO remove when ABI breaking changes are allowed.
+  throw runtime_error("Deprecated release operation", PI_INVALID_OPERATION);
 }
 
 void MemoryManager::releaseMemObj(ContextImplPtr TargetContext,
@@ -81,28 +81,10 @@ void *MemoryManager::allocate(ContextImplPtr TargetContext, SYCLMemObjI *MemObj,
                              OutEvent);
 }
 
-// Creates an image1d buffer wrapper object around given memory object.
 void *MemoryManager::wrapIntoImageBuffer(ContextImplPtr TargetContext,
                                          void *MemBuf, SYCLMemObjI *MemObj) {
-  // Image format: 1 channel per pixel, each pixel 8 bit, Size pixels occupies
-  // Size bytes.
-  pi_image_format Format = {PI_IMAGE_CHANNEL_ORDER_R,
-                            PI_IMAGE_CHANNEL_TYPE_UNSIGNED_INT8};
-
-  // Image descriptor - request wrapper image1d creation.
-  pi_image_desc Desc = {};
-  Desc.image_type = PI_MEM_TYPE_IMAGE1D_BUFFER;
-  Desc.image_width = MemObj->getSize();
-  Desc.buffer = reinterpret_cast<pi_mem>(MemBuf);
-
-  // Create the image object.
-  const detail::plugin &Plugin = TargetContext->getPlugin();
-  pi_mem Res = nullptr;
-  pi_mem_flags Flags = 0;
-  // Do not ref count the context handle, as it is not captured by the call.
-  Plugin.call<PiApiKind::piMemImageCreate>(TargetContext->getHandleRef(), Flags,
-                                           &Format, &Desc, nullptr, &Res);
-  return Res;
+  // TODO remove when ABI breaking changes are allowed.
+  throw runtime_error("Deprecated allocation operation", PI_INVALID_OPERATION);
 }
 
 void *MemoryManager::allocateHostMemory(SYCLMemObjI *MemObj, void *UserPtr,

sycl/source/detail/scheduler/commands.cpp

@@ -761,14 +761,6 @@ cl_int AllocaCommand::enqueueImp() {
       detail::getSyclObjImpl(MQueue->get_context()), getSYCLMemObj(),
       MInitFromUserData, HostPtr, std::move(EventImpls), Event);
 
-  // if this is ESIMD accessor, wrap the allocated device memory buffer into
-  // an image buffer object.
-  // TODO Address copying SYCL/ESIMD memory between contexts.
-  if (getRequirement()->MIsESIMDAcc)
-    ESIMDExt.MWrapperImage = MemoryManager::wrapIntoImageBuffer(
-        detail::getSyclObjImpl(MQueue->get_context()), MMemAllocation,
-        getSYCLMemObj());
-
   return CL_SUCCESS;
 }
 
@@ -960,10 +952,6 @@ cl_int ReleaseCommand::enqueueImp() {
                            MAllocaCmd->getSYCLMemObj(),
                            MAllocaCmd->getMemAllocation(),
                            std::move(EventImpls), Event);
-    // Release the wrapper object if present.
-    if (void *WrapperImage = MAllocaCmd->ESIMDExt.MWrapperImage)
-      MemoryManager::releaseImageBuffer(
-          detail::getSyclObjImpl(MQueue->get_context()), WrapperImage);
   }
   return CL_SUCCESS;
 }
@@ -1670,9 +1658,7 @@ pi_result ExecCGCommand::SetKernelParamsAndLaunch(
     case kernel_param_kind_t::kind_accessor: {
       Requirement *Req = (Requirement *)(Arg.MPtr);
       AllocaCommandBase *AllocaCmd = getAllocaForReq(Req);
-      RT::PiMem MemArg = Req->MIsESIMDAcc
-                             ? (RT::PiMem)AllocaCmd->ESIMDExt.MWrapperImage
-                             : (RT::PiMem)AllocaCmd->getMemAllocation();
+      RT::PiMem MemArg = (RT::PiMem)AllocaCmd->getMemAllocation();
       if (Plugin.getBackend() == backend::opencl) {
         Plugin.call<PiApiKind::piKernelSetArg>(Kernel, NextTrueIndex,
                                                sizeof(RT::PiMem), &MemArg);

sycl/source/detail/scheduler/commands.hpp

@@ -339,13 +339,6 @@ class AllocaCommandBase : public Command {
 
   void *MMemAllocation = nullptr;
 
-  // ESIMD-extension-specific fields.
-  struct {
-    // If this alloca corresponds to an ESIMD accessor, then this field holds
-    // an image buffer wrapping the memory allocation above.
-    void *MWrapperImage = nullptr;
-  } ESIMDExt;
-
   /// Alloca command linked with current command.
   /// Device and host alloca commands can be linked, so they may share the same
   /// memory. Only one allocation from a pair can be accessed at a time. Alloca

sycl/source/detail/scheduler/graph_builder.cpp

@@ -602,8 +602,7 @@ AllocaCommandBase *Scheduler::GraphBuilder::getOrCreateAllocaForReq(
       const Requirement FullReq(/*Offset*/ {0, 0, 0}, Req->MMemoryRange,
                                 Req->MMemoryRange, access::mode::read_write,
                                 Req->MSYCLMemObj, Req->MDims, Req->MElemSize,
-                                0 /*ReMOffsetInBytes*/, false /*MIsSubBuffer*/,
-                                Req->MIsESIMDAcc);
+                                0 /*ReMOffsetInBytes*/, false /*MIsSubBuffer*/);
       // Can reuse user data for the first allocation
       const bool InitFromUserData = Record->MAllocaCommands.empty();
 

sycl/source/handler.cpp

@@ -130,9 +130,17 @@ void handler::associateWithHandler(detail::AccessorBaseHost *AccBase,
                                    /*index*/ 0);
 }
 
+// TODO remove this one once ABI breaking changes are allowed.
 void handler::processArg(void *Ptr, const detail::kernel_param_kind_t &Kind,
                          const int Size, const size_t Index, size_t &IndexShift,
                          bool IsKernelCreatedFromSource) {
+  processArg(Ptr, Kind, Size, Index, IndexShift, IsKernelCreatedFromSource,
+             false);
+}
+
+void handler::processArg(void *Ptr, const detail::kernel_param_kind_t &Kind,
+                         const int Size, const size_t Index, size_t &IndexShift,
+                         bool IsKernelCreatedFromSource, bool IsESIMD) {
   using detail::kernel_param_kind_t;
 
   switch (Kind) {
@@ -162,7 +170,7 @@ void handler::processArg(void *Ptr, const detail::kernel_param_kind_t &Kind,
       // TODO ESIMD currently does not suport offset, memory and access ranges -
       // accessor::init for ESIMD-mode accessor has a single field, translated
       // to a single kernel argument set above.
-      if (!AccImpl->MIsESIMDAcc && !IsKernelCreatedFromSource) {
+      if (!IsKernelCreatedFromSource && !IsESIMD) {
         // Dimensionality of the buffer is 1 when dimensionality of the
         // accessor is 0.
         const size_t SizeAccField =
@@ -253,13 +261,21 @@ void handler::extractArgsAndReqs() {
     const detail::kernel_param_kind_t &Kind = UnPreparedArgs[I].MType;
     const int &Size = UnPreparedArgs[I].MSize;
     const int Index = UnPreparedArgs[I].MIndex;
-    processArg(Ptr, Kind, Size, Index, IndexShift, IsKernelCreatedFromSource);
+    processArg(Ptr, Kind, Size, Index, IndexShift, IsKernelCreatedFromSource,
+               false);
   }
 }
 
+// TODO remove once ABI breaking changes are allowed
 void handler::extractArgsAndReqsFromLambda(
     char *LambdaPtr, size_t KernelArgsNum,
     const detail::kernel_param_desc_t *KernelArgs) {
+  extractArgsAndReqsFromLambda(LambdaPtr, KernelArgsNum, KernelArgs, false);
+}
+
+void handler::extractArgsAndReqsFromLambda(
+    char *LambdaPtr, size_t KernelArgsNum,
+    const detail::kernel_param_desc_t *KernelArgs, bool IsESIMD) {
   const bool IsKernelCreatedFromSource = false;
   size_t IndexShift = 0;
   for (size_t I = 0; I < KernelArgsNum; ++I) {
@@ -284,7 +300,8 @@ void handler::extractArgsAndReqsFromLambda(
         Ptr = detail::getSyclObjImpl(*LocalAccBase).get();
       }
     }
-    processArg(Ptr, Kind, Size, I, IndexShift, IsKernelCreatedFromSource);
+    processArg(Ptr, Kind, Size, I, IndexShift, IsKernelCreatedFromSource,
+               IsESIMD);
   }
 }
 

sycl/test/abi/sycl_symbols_linux.dump

@@ -3830,10 +3830,12 @@ _ZN2cl4sycl7contextC2ERKSt6vectorINS0_6deviceESaIS3_EESt8functionIFvNS0_14except
 _ZN2cl4sycl7contextC2ERKSt8functionIFvNS0_14exception_listEEERKNS0_13property_listE
 _ZN2cl4sycl7contextC2ESt10shared_ptrINS0_6detail12context_implEE
 _ZN2cl4sycl7handler10processArgEPvRKNS0_6detail19kernel_param_kind_tEimRmb
+_ZN2cl4sycl7handler10processArgEPvRKNS0_6detail19kernel_param_kind_tEimRmbb
 _ZN2cl4sycl7handler13getKernelNameB5cxx11Ev
 _ZN2cl4sycl7handler18extractArgsAndReqsEv
 _ZN2cl4sycl7handler20associateWithHandlerEPNS0_6detail16AccessorBaseHostENS0_6access6targetE
 _ZN2cl4sycl7handler28extractArgsAndReqsFromLambdaEPcmPKNS0_6detail19kernel_param_desc_tE
+_ZN2cl4sycl7handler28extractArgsAndReqsFromLambdaEPcmPKNS0_6detail19kernel_param_desc_tEb
 _ZN2cl4sycl7handler6memcpyEPvPKvm
 _ZN2cl4sycl7handler6memsetEPvim
 _ZN2cl4sycl7handler7barrierERKSt6vectorINS0_5eventESaIS3_EE