[clang] [SYCL] The sycl_kernel_entry_point attribute. (PR #111389)

Tom Honermann via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 31 13:05:00 PDT 2024


================
@@ -14296,6 +14296,31 @@ void ASTContext::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
   }
 }
 
+static SYCLKernelInfo BuildSYCLKernelInfo(ASTContext &Context,
+                                          CanQualType KernelNameType,
+                                          const FunctionDecl *FD) {
+  return { KernelNameType, FD };
+}
+
+void ASTContext::registerSYCLEntryPointFunction(FunctionDecl *FD) {
+  assert(!FD->isInvalidDecl());
+  assert(!FD->isDependentContext());
+
+  const auto *SKEPAttr = FD->getAttr<SYCLKernelEntryPointAttr>();
+  assert(SKEPAttr && "Missing sycl_kernel_entry_point attribute");
+
+  CanQualType KernelNameType = getCanonicalType(SKEPAttr->getKernelName());
+  auto IT = SYCLKernels.find(KernelNameType);
+  if (IT != SYCLKernels.end()) {
+    if (!declaresSameEntity(FD, IT->second.GetKernelEntryPointDecl()))
+      llvm::report_fatal_error("SYCL kernel name conflict");
----------------
tahonermann wrote:

This PR doesn't provide enough functionality to demonstrate what would happen if Aaron's suggested recovery path was implemented. Nor do we have enough staged changes to easily perform a full end-to-end test. However, I was able to experiment with the set of changes we currently have staged [here](https://github.com/tahonermann/llvm-project/compare/main...tahonermann:llvm-project:sycl-upstream-fe-pr4). To do so, I removed the disputed call to `llvm::report_fatal_error()` (along with another one currently present in the linked staged changes) and the diagnostics that protect against that call and compiled the following test case.
```
template<typename KN, typename KT>
[[clang::sycl_kernel_entry_point(KN)]]
void skep(KT k) { k(); }
void f() {
  skep<struct K>([]{});
  skep<struct K>([]{});
}
```

Compilation was successful (it shouldn't have been of course; it succeeded because the diagnostics were elided). I then checked what symbols were emitted for device compilation. A little tangent first. As noted in the documentation added by this PR, ODR-use of a `sycl_kernel_entry_point` attributed function triggers emission of an offload kernel entry point function. The staged changes emit that function named as though it were a specialization of a function template named `__sycl_kernel_caller` with a single type template argument corresponding to the SYCL kernel name. Going into this experiment, I expected to end up with some kind of error or failed assertion related to an attempt to generate a duplicate symbol. But instead, some kind of function versioning kicked in.
```
$ nm t2.o
...
0000000000000020 T _Z20__sycl_kernel_callerIZ1fvE1KEvv
0000000000000040 T _Z20__sycl_kernel_callerIZ1fvE1KEvv.1
...
```

I'm not in a position to test with an actual SYCL run-time library, so I can only speculate here, but this looks like a scenario that I was worried about. Two entry point functions were emitted, but the SYCL run-time would only resolve the kernel name to one of them with the result that the wrong entry point would be silently invoked for some cases (had actual SYCL kernel invocation functions been used in the example). That would not be fun to debug.

We certainly can (and will) change our staged code to avoid the function versioning that is happening here; we should detect the situation and emit some kind of duplicate definition diagnostic or similar. With that kind of backup in place, I would feel more comfortable replacing the call to `llvm::report_fatal_error()` with an assert. But this is an example of why I have felt that we should do more to prevent such Bad Things from happening.

https://github.com/llvm/llvm-project/pull/111389


More information about the cfe-commits mailing list