Skip to content

Commit 2d36dde

Browse files
authored
[SYCL] Stop emission of modf intrinsic for NVPTX/AMDGCN (#17958)
A recent change in #126750 changed the lowering of the modf builtin(s) to lower to the llvm.modf intrinsic. Certain SYCL targets such as NVPTX and AMDGCN cannot currently handle this intrinsic in the backend because they don't have the necessary target library info. Even if they did, the SYCL device libraries are linked in earlier at the IR level so we'd be left with an unresolved symbol. We have been working around this at clang codegen time by avoiding the emission of intrinsics altogether by (ab)using a guard on math intrinsics. The problem with us hooking into GenerateIntrinsics like this is that SYCL is wanting to use it as a guarantee that (a certain subset of) intrinsics won't be emitted if GenerateIntrinsics is false, whereas upstream does not make this guarantee: it's an opt-in toggle for a more optimal lowering strategy. Thus we're always going to be liable to this sort of upstream change. This doesn't feel like the right mechanism for SYCL to handle these builtins (or intrinsics) long term, but this fix restores the previous behaviour without making things much worse. Fixes the AMDGCN/NVPTX aspects of #17813
1 parent a2d44f7 commit 2d36dde

File tree

5 files changed

+9
-8
lines changed

5 files changed

+9
-8
lines changed

clang/lib/CodeGen/CGBuiltin.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -3154,9 +3154,10 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
31543154
GenerateIntrinsics =
31553155
ConstWithoutErrnoOrExceptions && ErrnoOverridenToFalseWithOpt;
31563156
}
3157-
if (GenerateIntrinsics &&
3158-
!(getLangOpts().SYCLIsDevice && (getTarget().getTriple().isNVPTX() ||
3159-
getTarget().getTriple().isAMDGCN()))) {
3157+
bool IsSYCLDeviceWithoutIntrinsics =
3158+
getLangOpts().SYCLIsDevice &&
3159+
(getTarget().getTriple().isNVPTX() || getTarget().getTriple().isAMDGCN());
3160+
if (GenerateIntrinsics && !IsSYCLDeviceWithoutIntrinsics) {
31603161
switch (BuiltinIDIfNoAsmLabel) {
31613162
case Builtin::BIacos:
31623163
case Builtin::BIacosf:
@@ -4256,7 +4257,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
42564257
case Builtin::BI__builtin_modf:
42574258
case Builtin::BI__builtin_modff:
42584259
case Builtin::BI__builtin_modfl:
4259-
if (Builder.getIsFPConstrained())
4260+
if (Builder.getIsFPConstrained() || IsSYCLDeviceWithoutIntrinsics)
42604261
break; // TODO: Emit constrained modf intrinsic once one exists.
42614262
return RValue::get(emitModfBuiltin(*this, E, Intrinsic::modf));
42624263
case Builtin::BI__builtin_isgreater:

sycl/test-e2e/DeviceLib/cmath_fp64_test.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// UNSUPPORTED: true
1+
// UNSUPPORTED: spirv-backend
22
// UNSUPPORTED-TRACKER: https://github.com/intel/llvm/issues/17813
33
// REQUIRES: aspect-fp64
44
// UNSUPPORTED: target-amd || target-nvidia

sycl/test-e2e/DeviceLib/cmath_test.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// UNSUPPORTED: true
1+
// UNSUPPORTED: spirv-backend
22
// UNSUPPORTED-TRACKER: https://github.com/intel/llvm/issues/17813
33
// DEFINE: %{mathflags} = %if cl_options %{/clang:-fno-fast-math%} %else %{-fno-fast-math%}
44

sycl/test-e2e/DeviceLib/math_fp64_test.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// UNSUPPORTED: true
1+
// UNSUPPORTED: spirv-backend
22
// UNSUPPORTED-TRACKER: https://github.com/intel/llvm/issues/17813
33
// REQUIRES: aspect-fp64
44

sycl/test-e2e/DeviceLib/math_test.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// UNSUPPORTED: true
1+
// UNSUPPORTED: spirv-backend
22
// UNSUPPORTED-TRACKER: https://github.com/intel/llvm/issues/17813
33
// DEFINE: %{mathflags} = %if cl_options %{/clang:-fno-fast-math%} %else %{-fno-fast-math%}
44

0 commit comments

Comments
 (0)