Skip to content

[SYCL] Relax device code recursion ban for constexpr calls in constexpr context. #2104

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

Closed
kbobrovs opened this issue Jul 13, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@kbobrovs
Copy link
Contributor

SYCL spec restricts device code not to allow recursion.
In cases when recursion can be evaluated at compile-time, and the restriction can be lifted for programmer convenience. Even though the spec says

Recursion is not allowed in a SYCL kernel or any code called from the kernel

the intent is likely to reliably prevent recursive function code generation for the devices, the spec seems to need clarification.

With the proposed relaxation the following code should compile:

template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
  kernelFunc();
}

static constexpr unsigned int getNextPowerOf2(unsigned int n,
                                              unsigned int k = 1) {
  return (k >= n) ? k : getNextPowerOf2(n, k * 2);
}

unsigned test_constexpr_recursion(unsigned int val) {
  unsigned int res = val;
  unsigned int *addr = &res;

  kernel_single_task<class ConstexprRecursionKernel>([=]() {
    // Compiler must evaluate recursion, no errors expected.
    constexpr unsigned int x = getNextPowerOf2(3);
    *addr += x;
  });
  return res;
}

But if getNextPowerOf2 is called from non-constexpr context, it still should not compile:

template <typename name, typename Func>
__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
  kernelFunc(); //#call_kernelFunc // expected-note 3{{called by 'kernel_single_task<fake_kernel, (lambda at}}
}

static constexpr unsigned int getNextPowerOf2(unsigned int n,
                                              unsigned int k = 1) {
  return (k >= n) ? k : getNextPowerOf2(n, k * 2);
}

unsigned test_constexpr_recursion(unsigned int val) {
  unsigned int res = val;
  unsigned int *addr = &res;

  kernel_single_task<class ConstexprRecursionKernel>([=]() {
    // This call should still generate an error, as getNextPowerOf2
    //  will not be compile-time evaluated in this case and will be code-generated:
    unsigned int x = getNextPowerOf2(3);
    *addr += x;
  });
  return res;
}
@erichkeane
Copy link
Contributor

Note this is actually more complicated than just when assigning to a constexpr variable. Things like, the condition of a static assert, "constexpr if", array bounds, etc (basically, anywhere a constant expression is required!) should not diagnose if we are to allow this.

@AlexeySachkov AlexeySachkov added the enhancement New feature or request label Feb 2, 2021
jsji pushed a commit that referenced this issue Aug 11, 2023
It is a temporary change to enable back CI until we change LLVM version

Signed-off-by: Sidorov, Dmitry <[email protected]>

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@93b6d57
jsji pushed a commit that referenced this issue Aug 11, 2023
This reverts commit 93b6d57759a12875810a215ef781ccb0e928f374.

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@f80988f
@KornevNikita
Copy link
Contributor

Done by #2105.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants