[SYCL] Remove CG/handler extended members mechanism (#6759)

Now that the extended members have been promoted to proper fields of
CG/handler classes, the extended member mechanism can be removed until
it's needed again.

Author: sergey-semenov

sycl/doc/developer/ABIPolicyGuide.md

@@ -123,10 +123,3 @@ Whenever you need to change the existing ABI, please, follow these steps:
    part of version, as described above)
 
 At the end of this window we will increment major version of the DPC++ library
-
-## BKMs on avoiding changing ABI
-
-1. If there is a need to add a new field in `sycl::handler` or/and
-   `sycl::detail::CG` classes it can be done without breaking ABI using the
-   approach described in the comment at the beggining of
-   [cg.hpp](../../include/sycl/detail/cg.hpp)

sycl/include/sycl/detail/cg.hpp

@@ -42,111 +42,16 @@ namespace detail {
 class event_impl;
 using EventImplPtr = std::shared_ptr<event_impl>;
 
-// Periodically there is a need to extend handler and CG classes to hold more
-// data(members) than it has now. But any modification of the layout of those
-// classes is an ABI break. To have an ability to have more data the following
-// approach is implemented:
-//
-// Those classes have a member - MSharedPtrStorage which is an std::vector of
-// std::shared_ptr's and is supposed to hold reference counters of user
-// provided shared_ptr's.
-//
-// The first element of this vector is reused to store a vector of additional
-// members handler and CG need to have.
-//
-// These additional arguments are represented using "ExtendedMemberT" structure
-// which has a pointer to an arbitrary value and an integer which is used to
-// understand how the value the pointer points to should be interpreted.
-//
-// ========  ========      ========
-// |      |  |      | ...  |      | std::vector<std::shared_ptr<void>>
-// ========  ========      ========
-//    ||        ||            ||
-//    ||        \/            \/
-//    ||       user          user
-//    ||       data          data
-//    \/
-// ========  ========      ========
-// | Type |  | Type | ...  | Type | std::vector<ExtendedMemberT>
-// |      |  |      |      |      |
-// | Ptr  |  | Ptr  | ...  | Ptr  |
-// ========  ========      ========
-//
-// Prior to this change this vector was supposed to have user's values only, so
-// it is not legal to expect that the first argument is a special one.
-// Versioning is implemented to overcome this problem - if the first element of
-// the MSharedPtrStorage is a pointer to the special vector then CGType value
-// has version "1" encoded.
-//
-// The version of CG type is encoded in the highest byte of the value:
-//
-// 0x00000001 - CG type KERNEL version 0
-// 0x01000001 - CG type KERNEL version 1
-//    ^
-//    |
-// The byte specifies the version
-//
-// A user of this vector should not expect that a specific data is stored at a
-// specific position, but iterate over all looking for an ExtendedMemberT value
-// with the desired type.
-// This allows changing/extending the contents of this vector without changing
-// the version.
-
-// Used to represent a type of an extended member
-enum class ExtendedMembersType : unsigned int { PLACEHOLDER_TYPE };
-
-// Holds a pointer to an object of an arbitrary type and an ID value which
-// should be used to understand what type pointer points to.
-// Used as to extend handler class without introducing new class members which
-// would change handler layout.
-struct ExtendedMemberT {
-  ExtendedMembersType MType;
-  std::shared_ptr<void> MData;
-};
-
-static std::shared_ptr<std::vector<ExtendedMemberT>>
-convertToExtendedMembers(const std::shared_ptr<const void> &SPtr) {
-  return std::const_pointer_cast<std::vector<ExtendedMemberT>>(
-      std::static_pointer_cast<const std::vector<ExtendedMemberT>>(SPtr));
-}
-
 class stream_impl;
 class queue_impl;
 class kernel_bundle_impl;
 
-// The constant is used to left shift a CG type value to access it's version
-constexpr unsigned int ShiftBitsForVersion = 24;
-
-// Constructs versioned type
-constexpr unsigned int getVersionedCGType(unsigned int Type,
-                                          unsigned char Version) {
-  return Type | (static_cast<unsigned int>(Version) << ShiftBitsForVersion);
-}
-
-// Returns the type without version encoded
-constexpr unsigned char getUnversionedCGType(unsigned int Type) {
-  unsigned int Mask = -1;
-  Mask >>= (sizeof(Mask) * 8 - ShiftBitsForVersion);
-  return Type & Mask;
-}
-
-// Returns the version encoded to the type
-constexpr unsigned char getCGTypeVersion(unsigned int Type) {
-  return Type >> ShiftBitsForVersion;
-}
-
+// If there's a need to add new members to CG classes without breaking ABI
+// compatibility, we can bring back the extended members mechanism. See
+// https://github.com/intel/llvm/pull/6759
 /// Base class for all types of command groups.
 class CG {
 public:
-  // Used to version CG and handler classes. Using unsigned char as the version
-  // is encoded in the highest byte of CGType value. So it is not possible to
-  // encode a value > 255 anyway which should be big enough room for version
-  // bumping.
-  enum class CG_VERSION : unsigned char {
-    V0 = 0,
-    V1 = 1,
-  };
-
   /// Type of the command group.
   enum CGTYPE : unsigned int {
     None = 0,
@@ -189,21 +94,7 @@ class CG {
 
   CG(CG &&CommandGroup) = default;
 
-  CGTYPE getType() { return static_cast<CGTYPE>(getUnversionedCGType(MType)); }
-
-  CG_VERSION getVersion() {
-    return static_cast<CG_VERSION>(getCGTypeVersion(MType));
-  }
-
-  std::shared_ptr<std::vector<ExtendedMemberT>> getExtendedMembers() {
-    if (getCGTypeVersion(MType) == static_cast<unsigned int>(CG_VERSION::V0) ||
-        MSharedPtrStorage.empty())
-      return nullptr;
-
-    // The first value in shared_ptr storage is supposed to store a vector of
-    // extended members.
-    return convertToExtendedMembers(MSharedPtrStorage[0]);
-  }
+  CGTYPE getType() { return MType; }
 
   virtual ~CG() = default;
 

sycl/include/sycl/handler.hpp

@@ -380,15 +380,9 @@ class __SYCL_EXPORT handler {
     return Storage;
   }
 
-  void setType(detail::CG::CGTYPE Type) {
-    constexpr detail::CG::CG_VERSION Version = detail::CG::CG_VERSION::V1;
-    MCGType = static_cast<detail::CG::CGTYPE>(
-        getVersionedCGType(Type, static_cast<int>(Version)));
-  }
+  void setType(detail::CG::CGTYPE Type) { MCGType = Type; }
 
-  detail::CG::CGTYPE getType() {
-    return static_cast<detail::CG::CGTYPE>(getUnversionedCGType(MCGType));
-  }
+  detail::CG::CGTYPE getType() { return MCGType; }
 
   void throwIfActionIsCreated() {
     if (detail::CG::None != getType())

sycl/source/detail/global_handler.cpp

@@ -87,10 +87,6 @@ XPTIRegistry &GlobalHandler::getXPTIRegistry() {
   return getOrCreate(MXPTIRegistry);
 }
 
-std::mutex &GlobalHandler::getHandlerExtendedMembersMutex() {
-  return getOrCreate(MHandlerExtendedMembersMutex);
-}
-
 ThreadPool &GlobalHandler::getHostTaskThreadPool() {
   int Size = SYCLConfig<SYCL_QUEUE_THREAD_POOL_SIZE>::get();
   ThreadPool &TP = getOrCreate(MHostTaskThreadPool, Size);

sycl/source/detail/global_handler.hpp

@@ -67,7 +67,6 @@ class GlobalHandler {
   std::vector<plugin> &getPlugins();
   device_filter_list &getDeviceFilterList(const std::string &InitValue);
   XPTIRegistry &getXPTIRegistry();
-  std::mutex &getHandlerExtendedMembersMutex();
   ThreadPool &getHostTaskThreadPool();
 
   static void registerDefaultContextReleaseHandler();
@@ -101,8 +100,6 @@ class GlobalHandler {
   InstWithLock<std::vector<plugin>> MPlugins;
   InstWithLock<device_filter_list> MDeviceFilterList;
   InstWithLock<XPTIRegistry> MXPTIRegistry;
-  // The mutex for synchronizing accesses to handlers extended members
-  InstWithLock<std::mutex> MHandlerExtendedMembersMutex;
   // Thread pool for host task and event callbacks execution
   InstWithLock<ThreadPool> MHostTaskThreadPool;
 };

sycl/source/handler.cpp

@@ -37,12 +37,7 @@ handler::handler(std::shared_ptr<detail::queue_impl> Queue,
                  bool IsHost)
     : MImpl(std::make_shared<detail::handler_impl>(std::move(PrimaryQueue),
                                                    std::move(SecondaryQueue))),
-      MQueue(std::move(Queue)), MIsHost(IsHost) {
-  // Create extended members
-  auto ExtendedMembers =
-      std::make_shared<std::vector<detail::ExtendedMemberT>>();
-  MSharedPtrStorage.push_back(std::move(ExtendedMembers));
-}
+      MQueue(std::move(Queue)), MIsHost(IsHost) {}
 
 // Sets the submission state to indicate that an explicit kernel bundle has been
 // set. Throws a sycl::exception with errc::invalid if the current state