[SYCL][LIBCLC] Add memory/control barrier, sqrt, sin and cos builtins

[jchlanda]

No description provided.

[Naghasan]

Beside the mem_fence function name, LGTM

[bader]

@jchlanda, unfortunately ROCm plugin is not validated by CI system, so I have to clarify - there some in-tree tests or llvm-test-suite tests covering these built-ins. Right?

[jchlanda]

@jchlanda, unfortunately ROCm plugin is not validated by CI system, so I have to clarify - there some in-tree tests or llvm-test-suite tests covering these built-ins. Right?

@bader, yes there are tests for each built-in already in the repo, see ControlBarrier, or sqrt, admittedly, not the most robust.

[bader]

@jchlanda, unfortunately ROCm plugin is not validated by CI system, so I have to clarify - there some in-tree tests or llvm-test-suite tests covering these built-ins. Right?

@bader, yes there are tests for each built-in already in the repo, see ControlBarrier, or sqrt, admittedly, not the most robust.

AFAIK, libclc project lit tests are not executed in CI. :(
Buildbot CI builds checl-llvm, check-clang, checl-llvm-spirv and check-sycl targets. E.g. http://ci.llvm.intel.com:8010/#/builders/37/builds/9881.
Jenkins CI runs Khronos SYCL-CTS and SYCL tests from llvm-test-suite in addition BuildBot's list.

[Naghasan]

@jchlanda, unfortunately ROCm plugin is not validated by CI system, so I have to clarify - there some in-tree tests or llvm-test-suite tests covering these built-ins. Right?

@bader, yes there are tests for each built-in already in the repo, see ControlBarrier, or sqrt, admittedly, not the most robust.

AFAIK, libclc project lit tests are not executed in CI. :(
Buildbot CI builds checl-llvm, check-clang, checl-llvm-spirv and check-sycl targets. E.g. http://ci.llvm.intel.com:8010/#/builders/37/builds/9881.
Jenkins CI runs Khronos SYCL-CTS and SYCL tests from llvm-test-suite in addition BuildBot's list.

libclc lit tests only checks the binding produced will match what is expected from an SYCL perspective.
I added these libclc lit test mainly as sanity check before running the SYCL tests. Upstream libclc don't have one per say (unless it change since). The SYCL test suite is what ensures they work as expected.

Given the situation I agree it is better than nothing, but libclc tests are close to worthless anyhow (understand, you won't replace running on an actual device).

[bader]

I see that llvm-test-suite covers all these built-ins already (e.g. https://github.com/intel/llvm-test-suite/blob/intel/SYCL/DeviceLib/built-ins/scalar_math.cpp, https://github.com/intel/llvm-test-suite/blob/intel/SYCL/DeviceLib/cmath_test.cpp, ...).

@jchlanda, clang-format-check has failed. Will you apply clang-format to the patch or you have reasons to commit as is?

[jchlanda]

I see that llvm-test-suite covers all these built-ins already (e.g. https://github.com/intel/llvm-test-suite/blob/intel/SYCL/DeviceLib/built-ins/scalar_math.cpp, https://github.com/intel/llvm-test-suite/blob/intel/SYCL/DeviceLib/cmath_test.cpp, ...).

@jchlanda, clang-format-check has failed. Will you apply clang-format to the patch or you have reasons to commit as is?

@bader that was in a macro that I took from an existing file, my bad for not checking. Fixed now.

[bader]

Do we need SPIR-V built-in declarations? IIRC, @Naghasan added compiler support for these functions as compiler built-ins (can be enabled via clang's flag), so that they can be used w/o explicit declaration.

[Naghasan]

Do we need SPIR-V built-in declarations?

You are right, clang generates the builtin prototype internally, so this is not needed.

[jchlanda]

Removed, required a tweak to the way it's built (adding -Xclang -fdeclare-spirv-builtins).

[jchlanda]

@bader I can't access Jenkins/Precommit link, is that something that this patch has broken, or unrelated?

[bader]

It's not related.
@Naghasan, you've requested some changes. Does it look good to you now?

[Naghasan]

@Naghasan, you've requested some changes. Does it look good to you now?

yep, LGTM