Address review comments #1

Author: schittir

clang/include/clang/Basic/Attr.td

@@ -412,10 +412,6 @@ class LangOpt<string name, code customCode = [{}], bit silentlyIgnore = 0> {
   // The language option to test; ignored when custom code is supplied.
   string Name = name;
 
-  // If set to 1, the attribute is accepted but is silently ignored. This is
-  // useful in multi-compilation situations like SYCL.
-  bit SilentlyIgnore = silentlyIgnore;
-
   // A custom predicate, written as an expression evaluated in a context with
   // "LangOpts" bound.
   code CustomCode = customCode;
@@ -426,7 +422,6 @@ def CUDA : LangOpt<"CUDA">;
 def HIP : LangOpt<"HIP">;
 def SYCLHost : LangOpt<"SYCLIsHost">;
 def SYCLDevice : LangOpt<"SYCLIsDevice">;
-def SilentlyIgnoreSYCLHost : LangOpt<"SYCLIsHost", "", 1>;
 def COnly : LangOpt<"", "!LangOpts.CPlusPlus">;
 def CPlusPlus : LangOpt<"CPlusPlus">;
 def OpenCL : LangOpt<"OpenCL">;
@@ -1556,9 +1551,9 @@ def GlobalStorageNonLocalVar : SubsetSubject<Var,
                                              "global variables">;
 
 def SYCLExternal : InheritableAttr {
-  let Spellings = [GNU<"sycl_external">];
+  let Spellings = [Clang<"sycl_external">];
   let Subjects = SubjectList<[Function, GlobalStorageNonLocalVar]>;
-  let LangOpts = [SYCLDevice, SilentlyIgnoreSYCLHost];
+  let LangOpts = [SYCLDevice];
   let Documentation = [SYCLExternalDocs];
 }
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -12747,9 +12747,8 @@ def err_sycl_special_type_num_init_method : Error<
   "method defined">;
 
 //SYCL external attribute diagnostics
-def err_sycl_attribute_internal_decl
-    : Error<"%0 attribute cannot be applied to a %select{function|variable}1"
-            " without external linkage">;
+def err_sycl_attribute_invalid_linkage : Error<
+  "'sycl_external' can only be applied to functions with external linkage">;
 
 // SYCL kernel entry point diagnostics
 def err_sycl_entry_point_invalid : Error<

clang/include/clang/Sema/SemaSYCL.h

@@ -62,7 +62,7 @@ class SemaSYCL : public SemaBase {
                                        ParsedType ParsedTy);
 
   void handleKernelAttr(Decl *D, const ParsedAttr &AL);
-  void handleSYCLExternalAttr(Decl *D, const ParsedAttr &AL);
+  void handleExternalAttr(Decl *D, const ParsedAttr &AL);
   void handleKernelEntryPointAttr(Decl *D, const ParsedAttr &AL);
 
   void CheckSYCLEntryPointFunctionDecl(FunctionDecl *FD);

clang/lib/AST/ASTContext.cpp

@@ -12909,7 +12909,8 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
   if (D->hasAttr<WeakRefAttr>())
     return false;
 
-  if (LangOpts.SYCLIsDevice && !D->hasAttr<SYCLKernelEntryPointAttr>())
+  if (LangOpts.SYCLIsDevice &&
+       (!D->hasAttr<SYCLKernelEntryPointAttr>() || !D->hasAttr<SYCLExternalAttr>()))
     return false;
 
   // Aliases and used decls are required.
@@ -12927,10 +12928,9 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
     if (LangOpts.SYCLIsDevice && FD->hasAttr<SYCLKernelEntryPointAttr>())
       return true;
 
-    // FIXME: Functions declared with SYCL_EXTERNAL are required during
-    // device compilation.
-    // Functions definitions with sycl_external attribute are required during
-    // device compilation.
+    // Functions definitions with the sycl_external attribute are required
+    // during device compilation regardless of whether they are reachable from
+    // a SYCL kernel.
     if (LangOpts.SYCLIsDevice && FD->hasAttr<SYCLExternalAttr>())
       return true;
 

clang/lib/Sema/SemaDeclAttr.cpp

@@ -7114,7 +7114,7 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
     S.SYCL().handleKernelAttr(D, AL);
     break;
   case ParsedAttr::AT_SYCLExternal:
-    S.SYCL().handleSYCLExternalAttr(D, AL);
+    S.SYCL().handleExternalAttr(D, AL);
     break;
   case ParsedAttr::AT_SYCLKernelEntryPoint:
     S.SYCL().handleKernelEntryPointAttr(D, AL);

clang/lib/Sema/SemaSYCL.cpp

@@ -202,10 +202,10 @@ void SemaSYCL::handleKernelAttr(Decl *D, const ParsedAttr &AL) {
   handleSimpleAttribute<SYCLKernelAttr>(*this, D, AL);
 }
 
-void SemaSYCL::handleSYCLExternalAttr(Decl *D, const ParsedAttr &AL) {
+void SemaSYCL::handleExternalAttr(Decl *D, const ParsedAttr &AL) {
   auto *ND = cast<NamedDecl>(D);
   if (!ND->isExternallyVisible()) {
-    Diag(AL.getLoc(), diag::err_sycl_attribute_internal_decl)
+    Diag(AL.getLoc(), diag::err_sycl_attribute_invalid_linkage)
         << AL << !isa<FunctionDecl>(ND);
     return;
   }