[SYCL][CUDA] Change builtin selection for SYCL

[hdelan]

Libdevice for NVPTX was previously working due to the fact that LLVM intrinsics were not selected due to the CUDA toolchain having isMathErrnoDefault evaluate to true. Since LLVM intrinsics were not selected then the symbols could be found when linking with libdevice, which gives special backend specific definitions of CXX stdlib funcs.

Using errno to prevent intrinsic selection was a hack and it gets undone by using -ffast-math, meaning libdevice CXX funcs were not working with -ffast-math.

This change instead explicitly says not to use LLVM intrinsics if compiling SYCL for NVPTX backend. This means that -ffast-math behaviour should now be fixed for CXX stdlib funcs.

@jchlanda

[elizabethandrews]

Please add a test

[mdtoguchi]

OK for driver

[hdelan]

Please add a test

Test added

[elizabethandrews]

All FE changes should have an accompanying FE test. It makes it easier to track down issues in the future if one arises. Can you add a FE test for this change. It can just check these builtins are not present in IR with these options and present without.

[hdelan]

Sure. Test added. Let me know if you think this is sufficient. Note that SPIR-V compilation does use llvm intrinsics (not libdevice) for cmath funcs when in -ffast-math mode (since errno not used). This isn't exactly a problem for SPIR-V since llvm.cos.f32 has a SPIR-V lowering for fast mode, namely approx_cos. It happens to work but I think the idea of using libdevice is for DPC++ to provide its own definitions for cmath and other CXX stdlib funcs. Let me know if you think SPIR-V compilation should also use libdevice func definitions while in fast math mode as well, instead of relying on openCL driver implementations for say approx_cos.

[elizabethandrews]

Let me know if you think SPIR-V compilation should also use libdevice func definitions while in fast math mode as well, instead of relying on openCL driver implementations for say approx_cos.

I'm not familiar enough with this to have a non-naive response one way or the other. Maybe @bader or @steffenlarsen has an input?

[MrSidims]

Hi, sorry, missed the ping. There is no direct counterpart for the intrinsic in core SPIR-V. We can add translation of it to OpenCL's ldexp https://registry.khronos.org/SPIR-V/specs/1.0/OpenCL.ExtendedInstructionSet.100.mobile.html but unfortunately vector GPU compiler wouldn't be able to handle it as it appears to be not handling OpenCL.

So the first question would be: do you expect your patch to change compilation for ESIMD? If no - there is plenty of time for us to help you, since intrinsic to OpenCL builtin translation should be trivial (please create an internal feature request for that). If yes - probably we won't be able to help you for this release, from what I see now - it's quite hard to emulate such intrinsic replacing it with a sequence of SPIR-V instructions, so in this case it better to get rid of it before the translator.

[hdelan]

This patch is not changing compilation for SPIR-V targets at all, but the test I have added is highlighting a problem in LLVM-SPIR-V.

If I were to fix this for SPIR-V compilation it would merely avoid using LLVM intrinsics at all in SYCL, which is the default behaviour unless -ffast-math is used.

If we correctly map this intrinsic to an openCL builtin in LLVM-SPIR-V, then my understanding is that this would not work for say a L0 driver which as you say doesn't handle openCL SPIR-V. So we are shifting the problem elsewhere.

@steffenlarsen the long term problem is that libdevice doesn't do anything for ffast-math, maybe I should just disable the test for L0 at the moment, and add a TODO to add ffast-math functionality to libdevice. Once we have ffast-math working in libdevice then we can stop using LLVM intrinsics at all for SYCL SPIR-V compilation. Let me know your thoughts

[steffenlarsen]

Thank you for the clarification, @hdelan ! As long as it is not a regression, I am okay with disabling the test for targets we know don't support it, which I think in this case would be all SPIR-V targets, so maybe we should REQUIRE CUDA or HIP to be on the safe side here. Could you please open an issue here about this?

[hdelan]

Failures are unrelated. Ping @intel/llvm-gatekeepers this can be merged

[stdale-intel]

Failures are unrelated. Ping @intel/llvm-gatekeepers this can be merged

@hdelan , due to the failures that were happening (no gen12 linux tests run at all), we are not able to accept these test results as known issues. I have rekicked off your test run now that issue is fixed. If all comes back clean, gatekeepers will proceed with merge.

[dm-vodopyanov]

SYCL :: DeviceLib/cmath_test.cpp test failed, which was modified in this PR. @hdelan can you please take a look?

[hdelan]

@dm-vodopyanov that should hopefully fix it. Let's wait and see

[hdelan]

@dm-vodopyanov failures are unrelated.

[dm-vodopyanov]

Failed tests on HIP:

Failed Tests (2):
  SYCL :: AtomicRef/max_generic_local_native_fp.cpp
  SYCL :: GroupAlgorithm/leader.cpp

Logs:

$ "env" "ONEAPI_DEVICE_SELECTOR=ext_oneapi_hip:gpu" "/__w/llvm/llvm/build-e2e/AtomicRef/Output/max_generic_local_native_fp.cpp.tmp.out"
# command stderr:
Memory access fault by GPU node-1 (Agent handle: 0xd9[18](https://github.com/intel/llvm/actions/runs/5889588137/job/15974038334?pr=9768#step:22:19)c0) on address 0x7f93ec800000. Reason: Page not present or supervisor privilege.

error: command failed with exit status: -6

--

$ "env" "ONEAPI_DEVICE_SELECTOR=ext_oneapi_hip:gpu" "/__w/llvm/llvm/build-e2e/GroupAlgorithm/Output/leader.cpp.tmp.out"
# command stderr:
Memory access fault by GPU node-1 (Agent handle: 0x[23](https://github.com/intel/llvm/actions/runs/5889588137/job/15974038334?pr=9768#step:22:24)dea[70](https://github.com/intel/llvm/actions/runs/5889588137/job/15974038334?pr=9768#step:22:71)) on address 0x7fe8aa800000. Reason: Page not present or supervisor privilege.

error: command failed with exit status: -6