[SYCL] Addition of suggestions.

Signed-off-by: Garima Gupta <[email protected]>

Author: garimagu

sycl/include/CL/sycl/detail/pi.h

@@ -23,6 +23,10 @@
 #define _PI_H_VERSION_MAJOR 1
 #define _PI_H_VERSION_MINOR 1
 
+#define _PI_STRING_HELPER(a) #a
+#define _PI_CONCAT(a, b) _PI_STRING_HELPER(a.b)
+#define _PI_H_VERSION_STRING                                                   \
+  _PI_CONCAT(_PI_H_VERSION_MAJOR, _PI_H_VERSION_MINOR)
 // TODO: we need a mapping of PI to OpenCL somewhere, and this can be done
 // elsewhere, e.g. in the pi_opencl, but constants/enums mapping is now
 // done here, for efficiency and simplicity.
@@ -914,9 +918,6 @@ pi_result piEnqueueMemUnmap(
   pi_event *       event);
 
 
-#define STRING_HELPER(a) #a
-#define STRINGIZE(a,b) STRING_HELPER(a.b)
-
 struct _pi_plugin {
   // PI version supported by host passed to the plugin. The Plugin
   // checks and writes the appropriate Function Pointers in
@@ -925,19 +926,16 @@ struct _pi_plugin {
   // Some choices are:
   // - Use of integers to keep major and minor version.
   // - Keeping char* Versions.
-  const char PiVersion[4] = STRINGIZE(_PI_H_VERSION_MAJOR,_PI_H_VERSION_MINOR);
+  const char PiVersion[4] = _PI_H_VERSION_STRING; 
   // Plugin edits this.
-  char PluginVersion[4] = STRINGIZE(_PI_H_VERSION_MAJOR, _PI_H_VERSION_MINOR);
+  char PluginVersion[4] = _PI_H_VERSION_STRING;
   char *Targets;
   struct FunctionPointers {
 #define _PI_API(api) decltype(::api) *api;
 #include <CL/sycl/detail/pi.def>
   } PiFunctionTable;
 };
 
-#undef STRING_HELPER
-#undef STRINGIZE
-
 #ifdef __cplusplus
 } // extern "C"
 #endif // __cplusplus

sycl/include/CL/sycl/detail/pi.hpp

@@ -120,45 +120,68 @@ void printArgs(Arg0 arg0, Args... args) {
 // Utility function to check return from pi calls.
 // Throws if pi_result is not a PI_SUCCESS.
 template <typename Exception = cl::sycl::runtime_error>
-inline void piCheckResult(PiResult pi_result) {
+inline void checkPiResult(PiResult pi_result) {
   CHECK_OCL_CODE_THROW(pi_result, Exception);
 }
 
-#define PI_TRACE_ENABLED (std::getenv("SYCL_PI_TRACE") != nullptr)
-
-template <typename FnType, size_t FnOffset> class Trace {
+// Class to call PI API, trace and get the result.
+// To Trace : Set SYCL_PI_TRACE environment variable.
+// Template Arguments:
+//    FnType  - Type of Function pointer to the PI API.
+//    FnOffset- Offset to the Function Pointer in the piPlugin::FunctionPointers
+//    structure.Used to differentiate between APIs with same pointer type, Eg:
+//    piDeviceRelease and piDeviceRetain. Differentiation needed to avoid
+//    duplicate instantiation of class in pi.cpp.
+// Members: Initialiaed in default constructor in Class Template Specialization.
+// Usage:
+// Operator() - Call, Trace and Get result
+// Use Macro PI_CALL_NOCHECK call the constructor directly.
+template <typename FnType, size_t FnOffset> class CallPi {
 private:
-  FnType m_FnPtr;
-  std::string m_FnName;
+  FnType MFnPtr;
+  std::string MFnName;
+  static bool MEnableTrace;
 
 public:
-  Trace();
+  CallPi();
   template <typename... Args> PiResult operator()(Args... args) {
-    bool enableTrace = PI_TRACE_ENABLED;
-    if (enableTrace) {
-      std::cout << "---> " << m_FnName << "(";
+    if (MEnableTrace) {
+      std::cout << "---> " << MFnName << "(";
       printArgs(args...);
     }
 
-    PiResult r = m_FnPtr(args...);
+    PiResult r = MFnPtr(args...);
 
-    if (enableTrace) {
+    if (MEnableTrace) {
       std::cout << ") ---> ";
       std::cout << (print(r), "") << std::endl;
     }
     return r;
   }
 };
 
+template <typename FnType, size_t FnOffset>
+bool CallPi<FnType, FnOffset>::MEnableTrace = (std::getenv("SYCL_PI_TRACE") !=
+                                               nullptr);
+
+// Class to call PI API, trace, check the return result and throw Exception.
+// To Trace : Set SYCL_PI_TRACE environment variable.
+// Template Arguments:
+//    FnType, FnOffset - for CallPi Class.
+//    Exception - The type of exception to throw if PiResult of a call is not
+//    PI_SUCCESS. default is cl::sycl::runtime_error.
+// Usage:
+// Operator() - Call, Trace, check Result and Throw Exception.
+// Use Macro PI_CALL and PI_CALL_THROW to call the constructor directly.
 template <typename FnType, size_t FnOffset,
           typename Exception = cl::sycl::runtime_error>
-class TraceCheck : private Trace<FnType, FnOffset> {
+class CallPiAndCheck : private CallPi<FnType, FnOffset> {
 public:
-  TraceCheck() : Trace<FnType, FnOffset>(){};
+  CallPiAndCheck() : CallPi<FnType, FnOffset>(){};
 
   template <typename... Args> void operator()(Args... args) {
-    PiResult Err = (Trace<FnType, FnOffset>::operator()(args...));
-    piCheckResult<Exception>(Err);
+    PiResult Err = (CallPi<FnType, FnOffset>::operator()(args...));
+    checkPiResult<Exception>(Err);
   }
 };
 
@@ -167,39 +190,39 @@ class TraceCheck : private Trace<FnType, FnOffset> {
 // api.
 #define _PI_API(api)                                                           \
   template <>                                                                  \
-  Trace<decltype(&::api),                                                      \
-        (offsetof(pi_plugin::FunctionPointers, api))>::Trace();
+  CallPi<decltype(&::api),                                                     \
+         (offsetof(pi_plugin::FunctionPointers, api))>::CallPi();
 
 #include <CL/sycl/detail/pi.def>
 
 } // namespace pi
 
 namespace RT = cl::sycl::detail::pi;
 
-#define PI_ASSERT(cond, msg) RT::assertion((cond), "assert: " msg);
-
 // Use this macro to call the API, trace the call, check the return and throw a
 // runtime_error exception.
 // Usage: PI_CALL(pi)(Args);
 #define PI_CALL(pi)                                                            \
-  RT::TraceCheck<decltype(&::pi), (offsetof(pi_plugin::FunctionPointers, pi))>()
+  RT::CallPiAndCheck<decltype(&::pi), (offsetof(pi_plugin::FunctionPointers, pi))>()
 
 // Use this macro to call the API, trace the call and return the result.
-// To check the result use piCheckResult.
+// To check the result use checkPiResult.
 // Usage:
 // PiResult Err = PI_CALL_NOCHECK(pi)(args);
-// RT::piCheckResult(Err); <- Checks Result and throws a runtime error
+// RT::checkPiResult(Err); <- Checks Result and throws a runtime error
 // exception.
 #define PI_CALL_NOCHECK(pi)                                                    \
-  RT::Trace<decltype(&::pi), (offsetof(pi_plugin::FunctionPointers, pi))>()
+  RT::CallPi<decltype(&::pi), (offsetof(pi_plugin::FunctionPointers, pi))>()
 
 // Use this macro to call the API, trace the call, check the return and throw a
 // Exception as given in the MACRO.
 // Usage: PI_CALL_THROW(pi, compile_program_error)(args);
 #define PI_CALL_THROW(pi, Exception)                                           \
-  RT::TraceCheck<decltype(&::pi), (offsetof(pi_plugin::FunctionPointers, pi)), \
+  RT::CallPiAndCheck<decltype(&::pi), (offsetof(pi_plugin::FunctionPointers, pi)), \
                  Exception>()
 
+#define PI_ASSERT(cond, msg) RT::assertion((cond), "assert: " msg);
+
 // Want all the needed casts be explicit, do not define conversion
 // operators.
 template <class To, class From> To pi::cast(From value) {

sycl/include/CL/sycl/detail/program_impl.hpp

@@ -448,7 +448,7 @@ class program_impl {
         throw invalid_object_error(
             "This instance of program does not contain the kernel requested");
       }
-      RT::piCheckResult(Err);
+      RT::checkPiResult(Err);
     }
 
     return Kernel;

sycl/include/CL/sycl/detail/queue_impl.hpp

@@ -170,7 +170,7 @@ class queue_impl {
       m_SupportOOO = false;
       Queue = createQueue(QueueOrder::Ordered);
     } else {
-      RT::piCheckResult(Error);
+      RT::checkPiResult(Error);
     }
 
     return Queue;

sycl/plugins/opencl/pi_opencl.cpp

@@ -20,12 +20,7 @@
     return cast<pi_result>(reterr);                                            \
   }
 
-#define STRING_HELPER(a) #a
-#define STRINGIZE(a,b) STRING_HELPER(a.b)
-const char SupportedVersion[] = STRINGIZE(_PI_H_VERSION_MAJOR,_PI_H_VERSION_MINOR);
-#undef STRING_HELPER
-#undef STRINGIZE
-
+const char SupportedVersion[] = _PI_H_VERSION_STRING;
 
 // Want all the needed casts be explicit, do not define conversion operators.
 template <class To, class From> To cast(From value) {

sycl/source/detail/memory_manager.cpp

@@ -489,7 +489,7 @@ void MemoryManager::copy_usm(const void *SrcMem, QueueImplPtr SrcQueue,
 
     std::shared_ptr<usm::USMDispatcher> USMDispatch =
         getSyclObjImpl(Context)->getUSMDispatch();
-    RT::piCheckResult(USMDispatch->enqueueMemcpy(Queue,
+    RT::checkPiResult(USMDispatch->enqueueMemcpy(Queue,
                                                  /* blocking */ false, DstMem,
                                                  SrcMem, Len, DepEvents.size(),
                                                  &DepEvents[0], &OutEvent));
@@ -506,7 +506,7 @@ void MemoryManager::fill_usm(void *Mem, QueueImplPtr Queue, size_t Length,
   } else {
     std::shared_ptr<usm::USMDispatcher> USMDispatch =
         getSyclObjImpl(Context)->getUSMDispatch();
-    RT::piCheckResult(
+    RT::checkPiResult(
         USMDispatch->enqueueMemset(Queue->getHandleRef(), Mem, Pattern, Length,
                                    DepEvents.size(), &DepEvents[0], &OutEvent));
   }
@@ -522,7 +522,7 @@ void MemoryManager::prefetch_usm(void *Mem, QueueImplPtr Queue, size_t Length,
   } else {
     std::shared_ptr<usm::USMDispatcher> USMDispatch =
         getSyclObjImpl(Context)->getUSMDispatch();
-    RT::piCheckResult(USMDispatch->enqueuePrefetch(Queue->getHandleRef(), Mem,
+    RT::checkPiResult(USMDispatch->enqueuePrefetch(Queue->getHandleRef(), Mem,
                                                    Length, DepEvents.size(),
                                                    &DepEvents[0], &OutEvent));
   }

sycl/source/detail/pi.cpp

@@ -138,17 +138,14 @@ void assertion(bool Condition, const char *Message) {
 // TODO: Pass platform object to constructor which will contain the
 // PluginInformation class. Platform class with Plugin information is not
 // implemented yet.
-// Note: offsetof is used to differentiate between apis that have the same
-// signature. Eg: piDeviceRelease and piDeviceRetain.
-// offsetof(pi_plugin.PiFunctionTable,api)>
 
 #define _PI_API(api)                                                           \
   template <>                                                                  \
-  Trace<decltype(&::api),                                                      \
-        (offsetof(pi_plugin::FunctionPointers, api))>::Trace() {               \
+  CallPi<decltype(&::api),                                                      \
+        (offsetof(pi_plugin::FunctionPointers, api))>::CallPi() {               \
     initialize();                                                              \
-    m_FnPtr = (RT::PluginInformation.PiFunctionTable.api);                     \
-    m_FnName = #api;                                                           \
+    MFnPtr = (RT::PluginInformation.PiFunctionTable.api);                     \
+    MFnName = #api;                                                           \
   }
 #include <CL/sycl/detail/pi.def>
 

sycl/source/detail/sampler_impl.cpp

@@ -62,7 +62,7 @@ RT::PiSampler sampler_impl::getOrCreateSampler(const context &Context) {
   if (errcode_ret == PI_INVALID_OPERATION)
     throw feature_not_supported("Images are not supported by this device.");
 
-  RT::piCheckResult(errcode_ret);
+  RT::checkPiResult(errcode_ret);
   m_contextToSampler[Context] = resultSampler;
 
   return m_contextToSampler[Context];

sycl/source/detail/usm/usm_dispatch.cpp

@@ -301,7 +301,7 @@ pi_result USMDispatcher::enqueueMigrateMem(pi_queue Queue, const void *Ptr,
           CLQueue, NumEventsInWaitList,
           reinterpret_cast<const cl_event *>(EventWaitList),
           reinterpret_cast<cl_event *>(Event)));
-      pi::piCheckResult(RetVal);
+      pi::checkPiResult(RetVal);
     } else {
       RetVal = pi::cast<pi_result>(pfn_clEnqueueMigrateMemINTEL(
           CLQueue, Ptr, Size, Flags, NumEventsInWaitList,
@@ -349,19 +349,19 @@ void USMDispatcher::memAdvise(pi_queue Queue, const void *Ptr, size_t Length,
     if (mEmulated) {
       // memAdvise does nothing here
       // TODO: Implement a PI call for this openCL API
-      RT::piCheckResult(RT::cast<RT::PiResult>(clEnqueueMarkerWithWaitList(
+      RT::checkPiResult(RT::cast<RT::PiResult>(clEnqueueMarkerWithWaitList(
           CLQueue, 0, nullptr, reinterpret_cast<cl_event *>(Event))));
     } else {
       // Temporary until driver supports
       // memAdvise doesn't do anything on an iGPU anyway
       // TODO: Implement a PI call for this openCL API
-      RT::piCheckResult(RT::cast<RT::PiResult>(clEnqueueMarkerWithWaitList(
+      RT::checkPiResult(RT::cast<RT::PiResult>(clEnqueueMarkerWithWaitList(
           CLQueue, 0, nullptr, reinterpret_cast<cl_event *>(Event))));
       /*
       // Enable once this is supported in the driver
       auto CLAdvice = *reinterpret_cast<cl_mem_advice_intel *>(&Advice);
       // TODO: Implement a PI call for this openCL API
-      RT::piCheckResult(RT::cast<RT::PiResult>(pfn_clEnqueueMemAdviseINTEL(
+      RT::checkPiResult(RT::cast<RT::PiResult>(pfn_clEnqueueMemAdviseINTEL(
           CLQueue, Ptr, Length, CLAdvice, 0, nullptr,
           reinterpret_cast<cl_event *>(Event))));
       */

sycl/source/detail/usm/usm_impl.cpp

@@ -129,7 +129,7 @@ void free(void *Ptr, const context &Ctxt) {
     pi_context C = CtxImpl->getHandleRef();
     pi_result Error = Dispatch->memFree(C, Ptr);
 
-    RT::piCheckResult(Error);
+    RT::checkPiResult(Error);
   }
 }