Skip to content

[SYCL] Add diagnostic on command group with multiple actions #917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 10, 2019

Conversation

romanovvlad
Copy link
Contributor

Signed-off-by: Vlad Romanov [email protected]

@vladimirlaz
Copy link
Contributor

it is ok to have that check on run time for hard cases. But obvious cases may be detected and reported on compile time. Can we have that diagnostic in compiler?

@bader
Copy link
Contributor

bader commented Dec 9, 2019

it is ok to have that check on run time for hard cases. But obvious cases may be detected and reported on compile time. Can we have that diagnostic in compiler?

I don't see how this case can be detected at compile time. Do you have any ideas?

@vladimirlaz
Copy link
Contributor

it is ok to have that check on run time for hard cases. But obvious cases may be detected and reported on compile time. Can we have that diagnostic in compiler?

I don't see how this case can be detected at compile time. Do you have any ideas?

I was thinking about counting functions call of command group lambda in SemaSYCL. But it looks like it fill cause false alarms in case of branching in the lamda

vladimirlaz
vladimirlaz previously approved these changes Dec 9, 2019
@romanovvlad romanovvlad marked this pull request as ready for review December 9, 2019 16:30
@bader bader merged commit 9f8ae50 into intel:sycl Dec 10, 2019
vmaksimo pushed a commit to vmaksimo/llvm that referenced this pull request Mar 5, 2021
Removes excessive kernel code from the test examples. The key
source-level change for re-generating such IR from Intel SYCL is
to use `SYCL_EXTERNAL` function definitions instead of "true"
heterogeneous code. E.g. the following DPC++ code may be used to
generate IR for the AtomicFAdd test:

```
using namespace cl::sycl;

template <typename T>
using atomic_ref_T = ONEAPI::atomic_ref<T, ONEAPI::memory_order::relaxed,
                     ONEAPI::memory_scope::device,
                     access::address_space::global_space>;

SYCL_EXTERNAL decltype(auto) AtomicFloatInc(float &Arg) {
  atomic_ref_T<float> Atm(Arg);
  return Atm.fetch_add(1);
}

SYCL_EXTERNAL decltype(auto) AtomicDoubleInc(double &Arg) {
  atomic_ref_T<double> Atm(Arg);
  return Atm.fetch_add(1);
}

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@574d5bb
vladimirlaz pushed a commit that referenced this pull request Mar 30, 2021
Removes excessive kernel code from the test examples. The key
source-level change for re-generating such IR from Intel SYCL is
to use `SYCL_EXTERNAL` function definitions instead of "true"
heterogeneous code. E.g. the following DPC++ code may be used to
generate IR for the AtomicFAdd test:

```
using namespace cl::sycl;

template <typename T>
using atomic_ref_T = ONEAPI::atomic_ref<T, ONEAPI::memory_order::relaxed,
                     ONEAPI::memory_scope::device,
                     access::address_space::global_space>;

SYCL_EXTERNAL decltype(auto) AtomicFloatInc(float &Arg) {
  atomic_ref_T<float> Atm(Arg);
  return Atm.fetch_add(1);
}

SYCL_EXTERNAL decltype(auto) AtomicDoubleInc(double &Arg) {
  atomic_ref_T<double> Atm(Arg);
  return Atm.fetch_add(1);
}

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@574d5bb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants