[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

Wed Jan 18 07:53:13 PST 2023

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.

================
Comment at: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl:20
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone attribute.
+// FIXME: we don't, though.
 // CHECK-LABEL: @test_const_attr
----------------
svenvh wrote:
> lebedev.ri wrote:
> > I've looked, and i really don't understand how D64319 works.
> > It seems like the AST is then serialized into a header?
> > Because just adding a new attribute without spelling does not solve the issue.
> We're not setting the `NoUnwind` attribute for OpenCL (yet).  The following quick-and-dirty patch appears to fix this test for your patch (but will cause other tests to fail).  If you think it's time to add `NoUnwind` now, I can try putting up a review for that.
> 
> ```
> diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
> index 276d91fa2758..1ea3c11fbe03 100644
> --- a/clang/lib/CodeGen/CGCall.cpp
> +++ b/clang/lib/CodeGen/CGCall.cpp
> @@ -1972,7 +1972,7 @@ void CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
>    // TODO: NoUnwind attribute should be added for other GPU modes OpenCL, HIP,
>    // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
>    // code.
> -  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
> +  if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) || getLangOpts().OpenCL) {
>      // Exceptions aren't supported in CUDA device code.
>      FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
>    }
> ```
Yeah, that would be my guess as to how you'd fix this it, yes.
I'd suggest posting such a patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958