[SYCL][PI] Add versioning for plugins (#6216)

hdelan · web-flow · commit bf0fc4364d18 · 2022-06-17T11:43:14.000-07:00
Following on from #5412
diff --git a/sycl/include/CL/sycl/detail/pi.h b/sycl/include/CL/sycl/detail/pi.h
@@ -53,9 +53,28 @@
 
 #define _PI_STRING_HELPER(a) #a
 #define _PI_CONCAT(a, b) _PI_STRING_HELPER(a.b)
+#define _PI_TRIPLE_CONCAT(a, b, c) _PI_STRING_HELPER(a.b.c)
+
+// This is the macro that plugins should all use to define their version.
+// _PI_PLUGIN_VERSION_STRING will be printed when environment variable
+// SYCL_PI_TRACE is set to 1. PluginVersion should be defined for each plugin
+// in plugins/*/pi_*.hpp. PluginVersion should be incremented with each change
+// to the plugin.
+#define _PI_PLUGIN_VERSION_STRING(PluginVersion)                               \
+  _PI_TRIPLE_CONCAT(_PI_H_VERSION_MAJOR, _PI_H_VERSION_MINOR, PluginVersion)
+
 #define _PI_H_VERSION_STRING                                                   \
   _PI_CONCAT(_PI_H_VERSION_MAJOR, _PI_H_VERSION_MINOR)
 
+// This will be used to check the major versions of plugins versus the major
+// versions of PI.
+#define _PI_STRING_SUBSTITUTE(X) _PI_STRING_HELPER(X)
+#define _PI_PLUGIN_VERSION_CHECK(PI_API_VERSION, PI_PLUGIN_VERSION)            \
+  if (strncmp(PI_API_VERSION, PI_PLUGIN_VERSION,                               \
+              sizeof(_PI_STRING_SUBSTITUTE(_PI_H_VERSION_MAJOR))) < 0) {       \
+    return PI_ERROR_INVALID_OPERATION;                                         \
+  }
+
 // NOTE: This file presents a maping of OpenCL to PI enums, constants and
 // typedefs. The general approach taken was to replace `CL_` prefix with `PI_`.
 // Please consider this when adding or modifying values, as the strict value
@@ -1786,9 +1805,9 @@ struct _pi_plugin {
   // Some choices are:
   // - Use of integers to keep major and minor version.
   // - Keeping char* Versions.
-  char PiVersion[10];
+  char PiVersion[20];
   // Plugin edits this.
-  char PluginVersion[10];
+  char PluginVersion[20];
   char *Targets;
   struct FunctionPointers {
 #define _PI_API(api) decltype(::api) *api;
diff --git a/sycl/plugins/cuda/pi_cuda.cpp b/sycl/plugins/cuda/pi_cuda.cpp
@@ -5257,15 +5257,11 @@ pi_result cuda_piextUSMGetMemAllocInfo(pi_context context, const void *ptr,
 // pi_level_zero.cpp for reference) Currently this is just a NOOP.
 pi_result cuda_piTearDown(void *) { return PI_SUCCESS; }
 
-const char SupportedVersion[] = _PI_H_VERSION_STRING;
+const char SupportedVersion[] = _PI_CUDA_PLUGIN_VERSION_STRING;
 
 pi_result piPluginInit(pi_plugin *PluginInit) {
-  int CompareVersions = strcmp(PluginInit->PiVersion, SupportedVersion);
-  if (CompareVersions < 0) {
-    // PI interface supports lower version of PI.
-    // TODO: Take appropriate actions.
-    return PI_ERROR_INVALID_OPERATION;
-  }
+  // Check that the major version matches in PiVersion and SupportedVersion
+  _PI_PLUGIN_VERSION_CHECK(PluginInit->PiVersion, SupportedVersion);
 
   // PI interface supports higher version or the same version.
   size_t PluginVersionSize = sizeof(PluginInit->PluginVersion);
diff --git a/sycl/plugins/cuda/pi_cuda.hpp b/sycl/plugins/cuda/pi_cuda.hpp
@@ -18,6 +18,13 @@
 #ifndef PI_CUDA_HPP
 #define PI_CUDA_HPP
 
+// This version should be incremented for any change made to this file or its
+// corresponding .cpp file.
+#define _PI_CUDA_PLUGIN_VERSION 1
+
+#define _PI_CUDA_PLUGIN_VERSION_STRING                                         \
+  _PI_PLUGIN_VERSION_STRING(_PI_CUDA_PLUGIN_VERSION)
+
 #include "CL/sycl/detail/pi.h"
 #include <array>
 #include <atomic>
diff --git a/sycl/plugins/esimd_emulator/pi_esimd_emulator.cpp b/sycl/plugins/esimd_emulator/pi_esimd_emulator.cpp
@@ -1981,16 +1981,21 @@ pi_result piTearDown(void *) {
   return PI_SUCCESS;
 }
 
+const char SupportedVersion[] = _PI_ESIMD_PLUGIN_VERSION_STRING;
+
 pi_result piPluginInit(pi_plugin *PluginInit) {
   if (PluginInit == nullptr) {
     return PI_ERROR_INVALID_VALUE;
   }
 
+  // Check that the major version matches in PiVersion and SupportedVersion
+  _PI_PLUGIN_VERSION_CHECK(PluginInit->PiVersion, SupportedVersion);
+
   size_t PluginVersionSize = sizeof(PluginInit->PluginVersion);
   if (strlen(_PI_H_VERSION_STRING) >= PluginVersionSize) {
     return PI_ERROR_INVALID_VALUE;
   }
-  strncpy(PluginInit->PluginVersion, _PI_H_VERSION_STRING, PluginVersionSize);
+  strncpy(PluginInit->PluginVersion, SupportedVersion, PluginVersionSize);
 
   PiESimdDeviceAccess = new sycl::detail::ESIMDEmuPluginOpaqueData();
   // 'version' to be compared with 'ESIMD_EMULATOR_DEVICE_REQUIRED_VER' defined
diff --git a/sycl/plugins/esimd_emulator/pi_esimd_emulator.hpp b/sycl/plugins/esimd_emulator/pi_esimd_emulator.hpp
@@ -23,6 +23,13 @@
 
 #include <malloc.h>
 
+// This version should be incremented for any change made to this file or its
+// corresponding .cpp file.
+#define _PI_ESIMD_PLUGIN_VERSION 1
+
+#define _PI_ESIMD_PLUGIN_VERSION_STRING                                        \
+  _PI_PLUGIN_VERSION_STRING(_PI_ESIMD_PLUGIN_VERSION)
+
 namespace cm_support {
 #include <cm_rt.h>
 } // namespace cm_support
diff --git a/sycl/plugins/hip/pi_hip.cpp b/sycl/plugins/hip/pi_hip.cpp
@@ -4914,15 +4914,11 @@ pi_result hip_piTearDown(void *PluginParameter) {
   return PI_SUCCESS;
 }
 
-const char SupportedVersion[] = _PI_H_VERSION_STRING;
+const char SupportedVersion[] = _PI_HIP_PLUGIN_VERSION_STRING;
 
 pi_result piPluginInit(pi_plugin *PluginInit) {
-  int CompareVersions = strcmp(PluginInit->PiVersion, SupportedVersion);
-  if (CompareVersions < 0) {
-    // PI interface supports lower version of PI.
-    // TODO: Take appropriate actions.
-    return PI_ERROR_INVALID_OPERATION;
-  }
+  // Check that the major version matches in PiVersion and SupportedVersion
+  _PI_PLUGIN_VERSION_CHECK(PluginInit->PiVersion, SupportedVersion);
 
   // PI interface supports higher version or the same version.
   size_t PluginVersionSize = sizeof(PluginInit->PluginVersion);
diff --git a/sycl/plugins/hip/pi_hip.hpp b/sycl/plugins/hip/pi_hip.hpp
@@ -18,6 +18,13 @@
 #ifndef PI_HIP_HPP
 #define PI_HIP_HPP
 
+// This version should be incremented for any change made to this file or its
+// corresponding .cpp file.
+#define _PI_HIP_PLUGIN_VERSION 1
+
+#define _PI_HIP_PLUGIN_VERSION_STRING                                          \
+  _PI_PLUGIN_VERSION_STRING(_PI_HIP_PLUGIN_VERSION)
+
 #include "CL/sycl/detail/pi.h"
 #include <array>
 #include <atomic>
diff --git a/sycl/plugins/level_zero/pi_level_zero.cpp b/sycl/plugins/level_zero/pi_level_zero.cpp
@@ -8067,16 +8067,21 @@ pi_result piextProgramSetSpecializationConstant(pi_program Prog,
   return PI_SUCCESS;
 }
 
+const char SupportedVersion[] = _PI_LEVEL_ZERO_PLUGIN_VERSION_STRING;
+
 pi_result piPluginInit(pi_plugin *PluginInit) {
   PI_ASSERT(PluginInit, PI_ERROR_INVALID_VALUE);
 
+  // Check that the major version matches in PiVersion and SupportedVersion
+  _PI_PLUGIN_VERSION_CHECK(PluginInit->PiVersion, SupportedVersion);
+
   // TODO: handle versioning/targets properly.
   size_t PluginVersionSize = sizeof(PluginInit->PluginVersion);
 
-  PI_ASSERT(strlen(_PI_H_VERSION_STRING) < PluginVersionSize,
+  PI_ASSERT(strlen(_PI_LEVEL_ZERO_PLUGIN_VERSION_STRING) < PluginVersionSize,
             PI_ERROR_INVALID_VALUE);
 
-  strncpy(PluginInit->PluginVersion, _PI_H_VERSION_STRING, PluginVersionSize);
+  strncpy(PluginInit->PluginVersion, SupportedVersion, PluginVersionSize);
 
 #define _PI_API(api)                                                           \
   (PluginInit->PiFunctionTable).api = (decltype(&::api))(&api);
diff --git a/sycl/plugins/level_zero/pi_level_zero.hpp b/sycl/plugins/level_zero/pi_level_zero.hpp
@@ -18,6 +18,13 @@
 #ifndef PI_LEVEL_ZERO_HPP
 #define PI_LEVEL_ZERO_HPP
 
+// This version should be incremented for any change made to this file or its
+// corresponding .cpp file.
+#define _PI_LEVEL_ZERO_PLUGIN_VERSION 1
+
+#define _PI_LEVEL_ZERO_PLUGIN_VERSION_STRING                                   \
+  _PI_PLUGIN_VERSION_STRING(_PI_LEVEL_ZERO_PLUGIN_VERSION)
+
 #include <CL/sycl/detail/pi.h>
 #include <atomic>
 #include <cassert>
diff --git a/sycl/plugins/opencl/pi_opencl.cpp b/sycl/plugins/opencl/pi_opencl.cpp
@@ -18,6 +18,7 @@
 
 #include <CL/sycl/detail/cl.h>
 #include <CL/sycl/detail/pi.h>
+#include <pi_opencl.hpp>
 
 #include <algorithm>
 #include <cassert>
@@ -36,8 +37,6 @@
     return cast<pi_result>(reterr);                                            \
   }
 
-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) {
   // TODO: see if more sanity checks are possible.
@@ -1392,13 +1391,11 @@ pi_result piTearDown(void *PluginParameter) {
   return PI_SUCCESS;
 }
 
+const char SupportedVersion[] = _PI_OPENCL_PLUGIN_VERSION_STRING;
+
 pi_result piPluginInit(pi_plugin *PluginInit) {
-  int CompareVersions = strcmp(PluginInit->PiVersion, SupportedVersion);
-  if (CompareVersions < 0) {
-    // PI interface supports lower version of PI.
-    // TODO: Take appropriate actions.
-    return PI_ERROR_INVALID_OPERATION;
-  }
+  // Check that the major version matches in PiVersion and SupportedVersion
+  _PI_PLUGIN_VERSION_CHECK(PluginInit->PiVersion, SupportedVersion);
 
   // PI interface supports higher version or the same version.
   size_t PluginVersionSize = sizeof(PluginInit->PluginVersion);
diff --git a/sycl/plugins/opencl/pi_opencl.hpp b/sycl/plugins/opencl/pi_opencl.hpp
@@ -0,0 +1,27 @@
+//==---------- pi_opencl.hpp - OpenCL Plugin -------------------------------==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+/// \defgroup sycl_pi_ocl OpenCL Plugin
+/// \ingroup sycl_pi
+
+/// \file pi_opencl.hpp
+/// Declarations for vOpenCL Plugin. It is the interface between device-agnostic
+/// SYCL runtime layer and underlying OpenCL runtime.
+///
+/// \ingroup sycl_pi_ocl
+
+#ifndef PI_OPENCL_HPP
+#define PI_OPENCL_HPP
+
+// This version should be incremented for any change made to this file or its
+// corresponding .cpp file.
+#define _PI_OPENCL_PLUGIN_VERSION 1
+
+#define _PI_OPENCL_PLUGIN_VERSION_STRING                                       \
+  _PI_PLUGIN_VERSION_STRING(_PI_OPENCL_PLUGIN_VERSION)
+
+#endif // PI_OPENCL_HPP
diff --git a/sycl/source/detail/pi.cpp b/sycl/source/detail/pi.cpp
@@ -446,12 +446,14 @@ static void initializePlugins(std::vector<plugin> &Plugins) {
       GlobalPlugin = std::make_shared<plugin>(
           PluginInformation, backend::ext_intel_esimd_emulator, Library);
     }
-    Plugins.emplace_back(
+    plugin &NewPlugin = Plugins.emplace_back(
         plugin(PluginInformation, PluginNames[I].second, Library));
     if (trace(TraceLevel::PI_TRACE_BASIC))
       std::cerr << "SYCL_PI_TRACE[basic]: "
                 << "Plugin found and successfully loaded: "
-                << PluginNames[I].first << std::endl;
+                << PluginNames[I].first
+                << " [ PluginVersion: " << NewPlugin.getPiPlugin().PluginVersion
+                << " ]" << std::endl;
   }
 
 #ifdef XPTI_ENABLE_INSTRUMENTATION
