[clang][SYCL] Disable float128 device mode diagnostic #128513

npmiller · 2025-02-24T13:56:57Z

This diagnostic is disabled for device compilation as float128 is not supported on the device side.

Other diagnostics are already covering the cases where float128 is actually used in the kernel code, and it's already tested for in the existing test.

This is expanding on the patch 318bff6 that handled this for cuda compilation.

This diagnostic is disabled for device compilation as float128 is not supported on the device side. Other diagnostics are already covering the cases where float128 is actually used in the kernel code, and it's already tested for in the existing test. This is expanding on the patch 318bff6 that handled this for cuda compilation.

llvmbot · 2025-02-24T13:57:44Z

@llvm/pr-subscribers-clang

Author: Nicolas Miller (npmiller)

Changes

This diagnostic is disabled for device compilation as float128 is not supported on the device side.

Other diagnostics are already covering the cases where float128 is actually used in the kernel code, and it's already tested for in the existing test.

This is expanding on the patch 318bff6 that handled this for cuda compilation.

Full diff: https://github.com/llvm/llvm-project/pull/128513.diff

2 Files Affected:

(modified) clang/lib/Sema/SemaDeclAttr.cpp (+2-1)
(modified) clang/test/SemaSYCL/float128.cpp (+1)

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 620290af9509f..a13c04d69c4ba 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4700,7 +4700,8 @@ void Sema::AddModeAttr(Decl *D, const AttributeCommonInfo &CI,
 
   if (NewElemTy.isNull()) {
     // Only emit diagnostic on host for 128-bit mode attribute
-    if (!(DestWidth == 128 && getLangOpts().CUDAIsDevice))
+    if (!(DestWidth == 128 &&
+          (getLangOpts().CUDAIsDevice || getLangOpts().SYCLIsDevice)))
       Diag(AttrLoc, diag::err_machine_mode) << 1 /*Unsupported*/ << Name;
     return;
   }
diff --git a/clang/test/SemaSYCL/float128.cpp b/clang/test/SemaSYCL/float128.cpp
index b1a022216aaff..e41dea38dbe75 100644
--- a/clang/test/SemaSYCL/float128.cpp
+++ b/clang/test/SemaSYCL/float128.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple spir64 -fsycl-is-device -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -fsycl-is-device -fsyntax-only %s
 
+typedef _Complex float __cfloat128 __attribute__ ((__mode__ (__TC__)));
 typedef __float128 BIGTY;
 
 template <class T>

npmiller · 2025-02-24T13:59:01Z

clang/lib/Sema/SemaDeclAttr.cpp

@@ -4700,7 +4700,8 @@ void Sema::AddModeAttr(Decl *D, const AttributeCommonInfo &CI,

  if (NewElemTy.isNull()) {
    // Only emit diagnostic on host for 128-bit mode attribute
-    if (!(DestWidth == 128 && getLangOpts().CUDAIsDevice))
+    if (!(DestWidth == 128 &&
+          (getLangOpts().CUDAIsDevice || getLangOpts().SYCLIsDevice)))


Side-note CUDAIsDevice || SYCLIsDevice seems like a pretty common pattern and I believe HIP also uses CUDAIsDevice, it could be good to refactor in the future this to have a common "device compilation" option if possible.

On that note: I think the same problem can be reproduced by OpenMP offload as well, so we might need to extend the condition with || (getLangOpts().OpenMP && getLangOpts().OpenMPIsTargetDevice).

I'll try to reproduce this with OpenMP and open a follow up PR if it has the same issue

Not sure exactly how it works for OpenMP, but it doesn't seem to be affected, with a float128.cpp file just containing:

typedef _Complex float __cfloat128 __attribute__ ((__mode__ (__TC__)));

And testing without this patch:

With SYCL:

$ ./bin/clang++ float128.cpp -fsycl -fsycl-targets=nvptx64-nvidia-cuda -Xclang -fsycl-is-device -fsyntax-only -o o float128.cpp:1:52: error: unsupported machine mode '__TC__' 1 | typedef _Complex float __cfloat128 __attribute__ ((__mode__ (__TC__))); | ^ 1 error generated. $

With OpenMP:

$ ./bin/clang++ ../build/float128.cpp -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -Xclang -fopenmp-is-target-device -fsyntax-only -o o $

And same thing without specifying an OpenMP target.

Now, I'd like to understand how OpenMP compiler solves that problem and why OpenMP solution seems to be different from CUDA. @npmiller, do you know any reason why they should be different?

No I'm not very familiar with this part of the compiler, and even less with OpenMP.

ldrumm · 2025-03-03T16:08:43Z

I'm a bit confused about this.
First, the frontmatter for the repro:

$ gcc --version
gcc (Debian 14.2.0-17) 14.2.0
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

building intel/llvm@sycl

[12/53] Generating ../../lib/libsycl-fallback-complex.o
FAILED: lib/libsycl-fallback-complex.o /home/luke/src/upstream/llvm-sycl/build/lib/libsycl-fallback-complex.o 
cd /home/luke/src/upstream/llvm-sycl/build/tools/libdevice && /home/luke/src/upstream/llvm-sycl/build/bin/clang-20 -I /home/luke/src/upstream/llvm-sycl/build/include -Wno-sycl-strict -Wno-undefined-internal -sycl-std=2020 --target=x86_64-unknown-linux-gnu -fsycl -c -fsycl-targets=spir64_x86_64-unknown-unknown,spir64_gen-unknown-unknown,spir64-unknown-unknown,spirv64-unknown-unknown -fsycl -c -fsycl-targets=spir64_x86_64-unknown-unknown,spir64_gen-unknown-unknown,spir64-unknown-unknown,spirv64-unknown-unknown /home/luke/src/upstream/llvm-sycl/libdevice/fallback-complex.cpp -o /home/luke/src/upstream/llvm-sycl/build/./lib/libsycl-fallback-complex.o
In file included from /home/luke/src/upstream/llvm-sycl/libdevice/fallback-complex.cpp:12:
In file included from /home/luke/src/upstream/llvm-sycl/build/bin/../include/sycl/stl_wrappers/cmath:7:
In file included from /usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/cmath:47:
In file included from /usr/include/math.h:43:
/usr/include/x86_64-linux-gnu/bits/floatn.h:83:52: error: unsupported machine mode '__TC__'
   83 | typedef _Complex float __cfloat128 __attribute__ ((__mode__ (__TC__)));
      |                                                    ^
1 error generated.
[13/53] Generating ../../lib/libsycl-fallback-cmath-fp64.o
ninja: build stopped: subcommand failed.

At first I assumed that something had changed in libstdc++, but it turns out this line is actually in glibc and has been this way since 2017.
In the glibc tree, this is an x86-specific bits/ header: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/x86/bits/floatn.h#l83

Since this is happening in dpc++ and has been in glibc for a long time, I'm wondering what has changed. The present patch might be the right fix, but I'm not sure.

Perhaps @AaronBallman could suggest a reviewer (or review this himself) to decide whether this belongs here or has a better root-cause fix?

AaronBallman · 2025-03-04T22:01:19Z

I'm not certain if the fix is correct or not, so adding a few other reviewers who may have more confidence.

bader

The fix looks right to me.

bader · 2025-03-04T23:36:02Z

clang/lib/Sema/SemaDeclAttr.cpp

@@ -4700,7 +4700,8 @@ void Sema::AddModeAttr(Decl *D, const AttributeCommonInfo &CI,

  if (NewElemTy.isNull()) {
    // Only emit diagnostic on host for 128-bit mode attribute
-    if (!(DestWidth == 128 && getLangOpts().CUDAIsDevice))
+    if (!(DestWidth == 128 &&
+          (getLangOpts().CUDAIsDevice || getLangOpts().SYCLIsDevice)))


On that note: I think the same problem can be reproduced by OpenMP offload as well, so we might need to extend the condition with || (getLangOpts().OpenMP && getLangOpts().OpenMPIsTargetDevice).

Cherry-pick of: llvm/llvm-project#128513 Fixes: #16903 ------ This diagnostic is disabled for device compilation as float128 is not supported on the device side. Other diagnostics are already covering the cases where float128 is actually used in the kernel code, and it's already tested for in the existing test. This is expanding on the patch 318bff6 that handled this for cuda compilation.

This diagnostic is disabled for device compilation as float128 is not supported on the device side. Other diagnostics are already covering the cases where float128 is actually used in the kernel code, and it's already tested for in the existing test. This is expanding on the patch 318bff6 that handled this for cuda compilation.

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 24, 2025

npmiller commented Feb 24, 2025

View reviewed changes

AaronBallman requested review from bader, jdoerfert and erichkeane March 4, 2025 22:00

bader approved these changes Mar 4, 2025

View reviewed changes

bader mentioned this pull request Mar 10, 2025

SYCL compile error with glibc 2.41 intel/llvm#16903

Closed

ldrumm merged commit 7f415e4 into llvm:main Mar 12, 2025
14 checks passed

npmiller mentioned this pull request Mar 12, 2025

[clang][SYCL] Disable float128 device mode diagnostic (#128513) intel/llvm#17430

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang][SYCL] Disable float128 device mode diagnostic #128513

[clang][SYCL] Disable float128 device mode diagnostic #128513

Uh oh!

npmiller commented Feb 24, 2025

Uh oh!

llvmbot commented Feb 24, 2025

Uh oh!

npmiller Feb 24, 2025

Uh oh!

bader Mar 4, 2025

Uh oh!

npmiller Mar 12, 2025

Uh oh!

npmiller Mar 13, 2025

Uh oh!

bader Mar 13, 2025

Uh oh!

npmiller Mar 13, 2025

Uh oh!

ldrumm commented Mar 3, 2025

Uh oh!

AaronBallman commented Mar 4, 2025

Uh oh!

bader left a comment

Uh oh!

bader Mar 4, 2025

Uh oh!

Uh oh!

Uh oh!

[clang][SYCL] Disable float128 device mode diagnostic #128513

[clang][SYCL] Disable float128 device mode diagnostic #128513

Uh oh!

Conversation

npmiller commented Feb 24, 2025

Uh oh!

llvmbot commented Feb 24, 2025

Uh oh!

npmiller Feb 24, 2025

Choose a reason for hiding this comment

Uh oh!

bader Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

npmiller Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

npmiller Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

bader Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

npmiller Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

ldrumm commented Mar 3, 2025

Uh oh!

AaronBallman commented Mar 4, 2025

Uh oh!

bader left a comment

Choose a reason for hiding this comment

Uh oh!

bader Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!