From bf7c84c38a6f20f2abb10f12f19d627967ee7cd2 Mon Sep 17 00:00:00 2001
From: Steffen Larsen <steffen.larsen@intel.com>
Date: Sat, 29 Jun 2024 01:20:54 +0200
Subject: [PATCH] [SYCL] Simplify storePlainArg to avoid alias violations
 (#14344)

The helper function storePlainArg in handler and jit_compiler
reinterpret-casts pointers to memory in vectors of char in order to
store arguments in them. However, this violates strict aliasing and is
unnecessary as the resulting pointers are immediately converted to void*
after all calls to the function. As such, this patch simplfies these
implementations to always return void* and use memcpy to avoid the alias
violation.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
---
 sycl/include/sycl/handler.hpp       | 12 +++++-------
 sycl/source/detail/jit_compiler.cpp | 18 +++++++-----------
 2 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/sycl/include/sycl/handler.hpp b/sycl/include/sycl/handler.hpp
index a71f5400a813d..2b313c8834443 100644
--- a/sycl/include/sycl/handler.hpp
+++ b/sycl/include/sycl/handler.hpp
@@ -489,12 +489,10 @@ class __SYCL_EXPORT handler {
   handler(std::shared_ptr<ext::oneapi::experimental::detail::graph_impl> Graph);
 
   /// Stores copy of Arg passed to the CGData.MArgsStorage.
-  template <typename T, typename F = typename std::remove_const_t<
-                            typename std::remove_reference_t<T>>>
-  F *storePlainArg(T &&Arg) {
+  template <typename T> void *storePlainArg(T &&Arg) {
     CGData.MArgsStorage.emplace_back(sizeof(T));
-    auto Storage = reinterpret_cast<F *>(CGData.MArgsStorage.back().data());
-    *Storage = Arg;
+    void *Storage = static_cast<void *>(CGData.MArgsStorage.back().data());
+    std::memcpy(Storage, &Arg, sizeof(T));
     return Storage;
   }
 
@@ -691,7 +689,7 @@ class __SYCL_EXPORT handler {
   }
 
   template <typename T> void setArgHelper(int ArgIndex, T &&Arg) {
-    auto StoredArg = static_cast<void *>(storePlainArg(Arg));
+    void *StoredArg = storePlainArg(Arg);
 
     if (!std::is_same<cl_mem, T>::value && std::is_pointer<T>::value) {
       MArgs.emplace_back(detail::kernel_param_kind_t::kind_pointer, StoredArg,
@@ -703,7 +701,7 @@ class __SYCL_EXPORT handler {
   }
 
   void setArgHelper(int ArgIndex, sampler &&Arg) {
-    auto StoredArg = static_cast<void *>(storePlainArg(Arg));
+    void *StoredArg = storePlainArg(Arg);
     MArgs.emplace_back(detail::kernel_param_kind_t::kind_sampler, StoredArg,
                        sizeof(sampler), ArgIndex);
   }
diff --git a/sycl/source/detail/jit_compiler.cpp b/sycl/source/detail/jit_compiler.cpp
index e849fb3b57ad0..952482e42d79f 100644
--- a/sycl/source/detail/jit_compiler.cpp
+++ b/sycl/source/detail/jit_compiler.cpp
@@ -468,15 +468,6 @@ detectIdenticalParameter(std::vector<Param> &Params, ArgDesc Arg) {
   return Params.end();
 }
 
-template <typename T, typename F = typename std::remove_const_t<
-                          typename std::remove_reference_t<T>>>
-F *storePlainArg(std::vector<std::vector<char>> &ArgStorage, T &&Arg) {
-  ArgStorage.emplace_back(sizeof(T));
-  auto Storage = reinterpret_cast<F *>(ArgStorage.back().data());
-  *Storage = Arg;
-  return Storage;
-}
-
 void *storePlainArgRaw(std::vector<std::vector<char>> &ArgStorage, void *ArgPtr,
                        size_t ArgSize) {
   ArgStorage.emplace_back(ArgSize);
@@ -485,6 +476,11 @@ void *storePlainArgRaw(std::vector<std::vector<char>> &ArgStorage, void *ArgPtr,
   return Storage;
 }
 
+template <typename T>
+void *storePlainArg(std::vector<std::vector<char>> &ArgStorage, T &&Arg) {
+  return storePlainArgRaw(ArgStorage, &Arg, sizeof(T));
+}
+
 static ParamIterator preProcessArguments(
     std::vector<std::vector<char>> &ArgStorage, ParamIterator Arg,
     PromotionMap &PromotedAccs,
@@ -648,10 +644,10 @@ updatePromotedArgs(const ::jit_compiler::SYCLKernelInfo &FusedKernelInfo,
                                                          Req, Promotion::Local)
                                          : 0;
       range<3> AccessRange{1, 1, LocalSize};
-      auto *RangeArg = storePlainArg(FusedArgStorage, AccessRange);
+      void *RangeArg = storePlainArg(FusedArgStorage, AccessRange);
       // Use all-zero as the offset
       id<3> AcessOffset{0, 0, 0};
-      auto *OffsetArg = storePlainArg(FusedArgStorage, AcessOffset);
+      void *OffsetArg = storePlainArg(FusedArgStorage, AcessOffset);
 
       // Override the arguments.
       // 1. Override the pointer with a std-layout argument with 'nullptr' as