[SQUASH] Addressed review comments.

- Better checking and diagnostics
- More tests

Signed-off-by: Konstantin S Bobrovsky <[email protected]>

Author: kbobrovs

clang/include/clang/Basic/Attr.td

@@ -1135,16 +1135,6 @@ def SYCLKernel : InheritableAttr {
   let Documentation = [SYCLKernelDocs];
 }
 
-// Marks functions which must not be vectorized via horizontal SIMT widening,
-// e.g. because the function is already vectorized. Used to mark SYCL
-// explicit SIMD kernels and functions.
-def SYCLSimd : InheritableAttr {
-  let Spellings = [GNU<"sycl_explicit_simd">];
-  let Subjects = SubjectList<[Function]>;
-  let LangOpts = [SYCLIsDevice];
-  let Documentation = [SYCLSimdDocs];
-}
-
 // Available in SYCL explicit SIMD extension. Binds a file scope private
 // variable to a specific register.
 def SYCLRegisterNum : InheritableAttr {

clang/include/clang/Basic/AttrDocs.td

@@ -330,19 +330,6 @@ The SYCL kernel in the previous code sample meets these expectations.
   }];
 }
 
-def SYCLSimdDocs : Documentation {
-  let Category = DocCatFunction;
-  let Content = [{
-    The ``__attribute__((sycl_explicit_simd))`` attribute is used by the device
-    compiler front end to mark kernels and functions to be compiled and executed
-    in the explicit SIMD mode. In this mode subgroup size is always 1 and
-    explicit SIMD extensions - such as manual vectorization using wide vector
-    data types and operations can be used. Compiler may decide to compile such
-    functions using different different optimization and code generation
-    pipeline.
-  }];
-}
-
 def SYCLRegisterNumDocs : Documentation {
   let Category = DocCatVariable;
   let Content = [{

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10888,4 +10888,6 @@ def err_ext_int_bad_size : Error<"%select{signed|unsigned}0 _ExtInt must "
                                  "have a bit size of at least %select{2|1}0">;
 def err_ext_int_max_size : Error<"%select{signed|unsigned}0 _ExtInt of bit "
                                  "sizes greater than %1 not supported">;
+def err_esimd_glob_cant_init : Error<
+  "(SYCL explicit SIMD) private global variable cannot have an initializer">;
 } // end of sema component.

clang/include/clang/Sema/Sema.h

@@ -12660,6 +12660,14 @@ class Sema final {
   void finalizeSYCLDelayedAnalysis(const FunctionDecl *Caller,
                                    const FunctionDecl *Callee,
                                    SourceLocation Loc);
+
+  /// Tells whether given variable is a SYCL explicit SIMD extension's "private
+  /// global" variable - global variable in the private address space.
+  bool isSYCLEsimdPrivateGlobal(VarDecl *VDecl) {
+    return getLangOpts().SYCLIsDevice && getLangOpts().SYCLExplicitSIMD &&
+           VDecl->hasGlobalStorage() &&
+           (VDecl->getType().getAddressSpace() != LangAS::opencl_constant);
+  }
 };
 
 template <typename AttrType>

clang/lib/CodeGen/CodeGenModule.cpp

@@ -3885,13 +3885,6 @@ LangAS CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D) {
     if (!D || D->getType().getAddressSpace() == LangAS::Default) {
       return LangAS::opencl_global;
     }
-    if (D)
-      AddrSpace = D->getType().getAddressSpace();
-    if (AddrSpace == LangAS::opencl_private ||
-        AddrSpace == LangAS::opencl_local)
-      // SYCL explicit SIMD path: recognize globals in private or local address
-      // space.
-      return AddrSpace;
   }
 
   if (LangOpts.CUDA && LangOpts.CUDAIsDevice) {

clang/lib/Sema/SemaDecl.cpp

@@ -11979,6 +11979,13 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
     VDecl->setInvalidDecl();
     return;
   }
+  // In the SYCL explicit SIMD extension non constant "private globals" can't
+  // be explicitly initialized in the declaration.
+  if (isSYCLEsimdPrivateGlobal(VDecl)) {
+    Diag(VDecl->getLocation(), diag::err_esimd_glob_cant_init);
+    VDecl->setInvalidDecl();
+    return;
+  }
 
   // The LoaderUninitialized attribute acts as a definition (of undef).
   if (VDecl->hasAttr<LoaderUninitializedAttr>()) {
@@ -12578,12 +12585,9 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) {
     if (getLangOpts().OpenCL &&
         Var->getType().getAddressSpace() == LangAS::opencl_local)
       return;
-    // In SYCL ESIMD device code non-constant file scope variables can't be
-    // initialized.
-    // TODO add proper diagnostics for both SYCL and OpenCL paths
-    if (getLangOpts().SYCLExplicitSIMD && getLangOpts().SYCLIsDevice &&
-        Var->isFileVarDecl() && Var->hasGlobalStorage() &&
-        (Var->getType().getAddressSpace() != LangAS::opencl_constant))
+    // In SYCL explicit SIMD extension "private global" variables can't be
+    // initialized even implicitly, so don't synthesize an implicit initializer.
+    if (isSYCLEsimdPrivateGlobal(Var))
       return;
 
     // C++03 [dcl.init]p9:

clang/lib/Sema/SemaDeclAttr.cpp

@@ -4504,7 +4504,8 @@ static void handleSYCLRegisterNumAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
         << AL << 0;
     return;
   }
-  assert(AL.getNumArgs() == 1);
+  if (!checkAttributeNumArgs(S, AL, 1))
+    return;
   uint32_t RegNo = 0;
   const Expr *E = AL.getArgAsExpr(0);
   if (!checkUInt32Argument(S, AL, E, RegNo, 0, /*StrictlyUnsigned=*/true))

clang/lib/Sema/SemaExpr.cpp

@@ -221,8 +221,8 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,
       if (!IsConst && VD->getStorageClass() == SC_Static)
         SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict)
             << Sema::KernelNonConstStaticDataVariable;
-      // Non-const globals are allowed for CM.
-      else if (!getLangOpts().SYCLExplicitSIMD && !IsConst &&
+      // Non-const globals are allowed for SYCL explicit SIMD.
+      else if (!isSYCLEsimdPrivateGlobal(VD) && !IsConst &&
                VD->hasGlobalStorage() && !isa<ParmVarDecl>(VD))
         SYCLDiagIfDeviceCode(*Locs.begin(), diag::err_sycl_restrict)
             << Sema::KernelGlobalVariable;

clang/test/CodeGenSYCL/esimd-private-global.cpp

@@ -5,10 +5,10 @@
 // This test checks that FE allows globals with register_num attribute in ESIMD mode.
 
 __attribute__((opencl_private)) __attribute__((register_num(17))) int vc;
-
-// CHECK-DAG: @vc = {{.+}} i32 0, align 4 #0
-// CHECK-DAG: attributes #0 = { "genx_byte_offset"="17" "genx_volatile" }
+// CHECK: @vc = {{.+}} i32 0, align 4 #0
 
 SYCL_EXTERNAL void init_vc(int x) {
   vc = x;
+  // CHECK: store i32 %0, i32* @vc
 }
+// CHECK: attributes #0 = { "genx_byte_offset"="17" "genx_volatile" }

clang/test/SemaSYCL/esimd-private-global.cpp

@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsycl -fsycl-is-device -fsycl-explicit-simd -fsyntax-only -verify -pedantic %s
+
+// no error expected
+__attribute__((opencl_private)) __attribute__((register_num(17))) int privGlob;
+
+// expected-error@+1{{'register_num' attribute takes one argument}}
+__attribute__((opencl_private)) __attribute__((register_num())) int privGlob1;
+
+// expected-error@+1{{'register_num' attribute takes one argument}}
+__attribute__((opencl_private)) __attribute__((register_num(10, 11))) int privGlob2;
+
+// expected-error@+1{{(SYCL explicit SIMD) private global variable cannot have an initializer}}
+__attribute__((opencl_private)) int privGlob3 = 10;
+
+void foo() {
+  // expected-warning@+1{{'register_num' attribute only applies to global variables}}
+  __attribute__((register_num(17))) int privLoc;
+}