Skip to content

Commit

Permalink
[SYCL] Simplify storePlainArg to avoid alias violations (#14344)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
steffenlarsen committed Jun 28, 2024
1 parent deeb664 commit bf7c84c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 18 deletions.
12 changes: 5 additions & 7 deletions sycl/include/sycl/handler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
Expand Down
18 changes: 7 additions & 11 deletions sycl/source/detail/jit_compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit bf7c84c

Please sign in to comment.