[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Harald van Dijk via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6b868139458d: [SYCL] Always set NoUnwind attribute for SYCL. 
(authored by hvdijk).

Changed prior to commit:
  https://reviews.llvm.org/D147097?vs=509355=509528#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147097/new/

https://reviews.llvm.org/D147097

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenSYCL/convergent.cpp
  clang/test/CodeGenSYCL/function-attrs.cpp


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
+
+int foo();
+
+// CHECK: define dso_local spir_func void @_Z3barv() [[BAR:#[0-9]+]]
+// CHECK: attributes [[BAR]] =
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+void bar() {
+  int a = foo();
+}
+
+int foo() {
+  return 1;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(const Func ) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([] { bar(); });
+  return 0;
+}
Index: clang/test/CodeGenSYCL/convergent.cpp
===
--- clang/test/CodeGenSYCL/convergent.cpp
+++ /dev/null
@@ -1,19 +0,0 @@
-// RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
-
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
-void foo() {
-  int a = 1;
-}
-
-template 
-__attribute__((sycl_kernel)) void kernel_single_task(const Func ) {
-  kernelFunc();
-}
-
-int main() {
-  kernel_single_task([] { foo(); });
-  return 0;
-}
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
+
+int foo();
+
+// CHECK: define dso_local spir_func void @_Z3barv() [[BAR:#[0-9]+]]
+// CHECK: attributes [[BAR]] =
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+void bar() {
+  int a = foo();
+}
+
+int foo() {
+  return 1;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(const Func ) {
+  kernelFunc();
+}
+
+int main() {
+  kernel_single_task([] { bar(); });
+  return 0;
+}
Index: clang/test/CodeGenSYCL/convergent.cpp
===
--- clang/test/CodeGenSYCL/convergent.cpp
+++ /dev/null
@@ -1,19 +0,0 @@
-// RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
-
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
-void foo() {
-  int a = 1;
-}
-
-template 
-__attribute__((sycl_kernel)) void kernel_single_task(const Func ) {
-  kernelFunc();
-}
-
-int main() {
-  kernel_single_task([] { foo(); });
-  return 0;
-}
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Alexey Bader via Phabricator via cfe-commits]

bader accepted this revision.
bader added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147097/new/

https://reviews.llvm.org/D147097

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk updated this revision to Diff 509355.
hvdijk added a comment.

Forgot to update the call to `foo()` to call `bar()` instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147097/new/

https://reviews.llvm.org/D147097

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenSYCL/convergent.cpp
  clang/test/CodeGenSYCL/function-attrs.cpp


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- clang/test/CodeGenSYCL/function-attrs.cpp
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -1,11 +1,18 @@
 // RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
 
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
-void foo() {
-  int a = 1;
+int foo();
+
+// CHECK: define dso_local spir_func void @_Z3barv() [[BAR:#[0-9]+]]
+// CHECK: attributes [[BAR]] =
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+void bar() {
+  int a = foo();
+}
+
+int foo() {
+  return 1;
 }
 
 template 
@@ -14,6 +21,6 @@
 }
 
 int main() {
-  kernel_single_task([] { foo(); });
+  kernel_single_task([] { bar(); });
   return 0;
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- clang/test/CodeGenSYCL/function-attrs.cpp
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -1,11 +1,18 @@
 // RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
 
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
-void foo() {
-  int a = 1;
+int foo();
+
+// CHECK: define dso_local spir_func void @_Z3barv() [[BAR:#[0-9]+]]
+// CHECK: attributes [[BAR]] =
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+void bar() {
+  int a = foo();
+}
+
+int foo() {
+  return 1;
 }
 
 template 
@@ -14,6 +21,6 @@
 }
 
 int main() {
-  kernel_single_task([] { foo(); });
+  kernel_single_task([] { bar(); });
   return 0;
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk updated this revision to Diff 509335.
hvdijk added a comment.

Apparently the test was already passing without my change, so I updated the 
test to make it a function that did not previously get the nounwind attribute. 
This also revealed that the test was more fragile than I realised: it depends 
on the precise order in which functions are emitted. We always match "Function 
Attrs:" to the first emitted function, and then `CHECK-NEXT` can fail. We'd 
ideally first `CHECK`  for `void @_Z3foov()`  (now `void⋅@_Z3barv()`) and then 
`CHECK-PREV`  for the attributes, but FileCheck does not support that. What we 
can do instead though is find the matching `attributes #n = { ... }`  line and 
verify that everything is in there; updated accordingly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147097/new/

https://reviews.llvm.org/D147097

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenSYCL/convergent.cpp
  clang/test/CodeGenSYCL/function-attrs.cpp


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- clang/test/CodeGenSYCL/function-attrs.cpp
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -1,11 +1,18 @@
 // RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
 
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
-void foo() {
-  int a = 1;
+int foo();
+
+// CHECK: define dso_local spir_func void @_Z3barv() [[BAR:#[0-9]+]]
+// CHECK: attributes [[BAR]] =
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+void bar() {
+  int a = foo();
+}
+
+int foo() {
+  return 1;
 }
 
 template 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- clang/test/CodeGenSYCL/function-attrs.cpp
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -1,11 +1,18 @@
 // RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
 
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
-void foo() {
-  int a = 1;
+int foo();
+
+// CHECK: define dso_local spir_func void @_Z3barv() [[BAR:#[0-9]+]]
+// CHECK: attributes [[BAR]] =
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+void bar() {
+  int a = foo();
+}
+
+int foo() {
+  return 1;
 }
 
 template 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

> That's a good question. I haven't looked into this issue deep enough, but I 
> think using -fexceptions requires using delayed diagnostics to avoid false 
> diagnostics during host code analysis.

I am assuming you mean `-fno-exceptions` (or, in `clang -cc1`, the absence of 
`-fexceptions`)? This is a good point. Delayed diagnostics would probably be 
good in general: we currently already emit warnings for host code when 
compiling for device, but as long as the generated warnings are identical for 
the host code as when compiling for host, it is easy enough to ignore; if the 
device compilation were to result in additional warnings for host code, and 
those warnings are incorrect, that would be a rather poor user experience. That 
sounds like a good additional reason to do it the way I had done here, the way 
the existing code comment had indicated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147097/new/

https://reviews.llvm.org/D147097

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

@hvdijk, thanks a lot for fixing this.

In D147097#4229121 , @hvdijk wrote:

> Is the rationale I gave in the description correct, or would it be better for 
> SYCL device code to unconditionally build without `-fexceptions` and get the 
> `nounwind` attribute added that way?

That's a good question. I haven't looked into this issue deep enough, but I 
think using `-fexceptions` requires using delayed diagnostics to avoid false 
diagnostics during host code analysis. 
Anyway, all GPU offloading single-source modes have the same restriction and 
design and we better have unified solution whether it's using `-fexceptions` or 
adding `nounwind` attribute in CodeGen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147097/new/

https://reviews.llvm.org/D147097

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

Is the rationale I gave in the description correct, or would it be better for 
SYCL device code to unconditionally build without `-fexceptions` and get the 
`nounwind` attribute added that way?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147097/new/

https://reviews.llvm.org/D147097

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk created this revision.
hvdijk added a reviewer: bader.
hvdijk added a project: clang.
Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl.
Herald added a project: All.
hvdijk requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.

Like CUDA and OpenCL, the SYCL specification says that throwing and
catching exceptions in device functions is not supported, so this change
extends the logic for adding the NoUnwind attribute to SYCL.

The existing convergent.cpp test, which tests that the convergent
attribute is added to functions by default, is renamed and reused to
test that the nounwind attribute is added by default. This test now has
-fexceptions added to it, which the driver adds by default as well.

The obvious question here is why not simply change the driver to remove
-fexceptions. This change follows the direction given by the TODO
comment because removing -fexceptions would also disable the
__EXCEPTIONS macro, which should reflect whether exceptions are enabled
on the host, rather than on the device, to avoid conflicts in types
shared between host and device.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147097

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenSYCL/convergent.cpp
  clang/test/CodeGenSYCL/function-attrs.cpp


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- clang/test/CodeGenSYCL/function-attrs.cpp
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -1,9 +1,10 @@
 // RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
 
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
+// CHECK: Function Attrs:
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+// CHECK-NEXT: define dso_local spir_func void @_Z3foov
 void foo() {
   int a = 1;
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 


Index: clang/test/CodeGenSYCL/function-attrs.cpp
===
--- clang/test/CodeGenSYCL/function-attrs.cpp
+++ clang/test/CodeGenSYCL/function-attrs.cpp
@@ -1,9 +1,10 @@
 // RUN: %clang_cc1 -fsycl-is-device -emit-llvm -disable-llvm-passes \
-// RUN:  -triple spir64 -emit-llvm %s -o - | FileCheck %s
+// RUN:  -triple spir64 -fexceptions -emit-llvm %s -o - | FileCheck %s
 
-// CHECK-DAG: Function Attrs:
-// CHECK-DAG-SAME: convergent
-// CHECK-DAG-NEXT: define void @_Z3foov
+// CHECK: Function Attrs:
+// CHECK-SAME: convergent
+// CHECK-SAME: nounwind
+// CHECK-NEXT: define dso_local spir_func void @_Z3foov
 void foo() {
   int a = 1;
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1971,10 +1971,9 @@
   }
 
   // TODO: NoUnwind attribute should be added for other GPU modes HIP,
-  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
-  // code.
+  // OpenMP offload. AFAIK, neither of them support exceptions in device code.
   if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
-  getLangOpts().OpenCL) {
+  getLangOpts().OpenCL || getLangOpts().SYCLIsDevice) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits