[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-04-04 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC357684: [PR41157][OpenCL] Prevent implicit init of local 
addr space var in C++ mode. (authored by stulova, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D59646?vs=193474=193686#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59646

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl


Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -150,15 +150,13 @@
 TEST(__local)
 
 // COMMON-LABEL: _Z11test__localv
-// EXPL: @__cxa_guard_acquire
 
-// Test the address space of 'this' when invoking a constructor for an object 
in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an object in local address space.
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking a method.
 // COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* 
addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C 
addrspace(4)*))
 
-
 // Test the address space of 'this' when invoking copy-constructor.
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C 
addrspace(4)*
 // EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], 
%class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)* 
@_ZZ11test__localvE1c to %class.C addrspace(4)*))
Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- test/CodeGenOpenCLCXX/local_addrspace_init.cl
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck 
%s
+
+// Test that we don't initialize local address space objects.
+//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+class C {
+  int i;
+};
+
+kernel void test() {
+  __local int i;
+  __local C ii;
+  // FIXME: In OpenCL C we don't accept initializers for local
+  // address space variables. User defined initialization could
+  // make sense, but would it mean that all work items need to
+  // execute it? Potentially disallowing any initialization would
+  // make things easier and assingments can be used to set specific
+  // values. This rules should make it consistent with OpenCL C.
+  //__local C c();
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11682,7 +11682,11 @@
   setFunctionHasBranchProtectedScope();
   }
 }
-
+// In OpenCL, we can't initialize objects in the __local address space,
+// even implicitly, so don't synthesize an implicit initializer.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;
 // C++03 [dcl.init]p9:
 //   If no initializer is specified for an object, and the
 //   object is of (possibly cv-qualified) non-POD class type (or


Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -150,15 +150,13 @@
 TEST(__local)
 
 // COMMON-LABEL: _Z11test__localv
-// EXPL: @__cxa_guard_acquire
 
-// Test the address space of 'this' when invoking a constructor for an object in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an object in local address space.
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking a method.
 // COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
 
-
 // Test the address space of 'this' when invoking copy-constructor.
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
 // EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)*

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-04-03 Thread John McCall via Phabricator via cfe-commits 
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.


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

https://reviews.llvm.org/D59646



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-04-03 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia updated this revision to Diff 193474.
Anastasia added a comment.

Improved comment about initializers in __local addr space.


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

https://reviews.llvm.org/D59646

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl


Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck 
%s
+
+// Test that we don't initialize local address space objects.
+//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+class C {
+  int i;
+};
+
+kernel void test() {
+  __local int i;
+  __local C ii;
+  // FIXME: In OpenCL C we don't accept initializers for local
+  // address space variables. User defined initialization could
+  // make sense, but would it mean that all work items need to
+  // execute it? Potentially disallowing any initialization would
+  // make things easier and assingments can be used to set specific
+  // values. This rules should make it consistent with OpenCL C.
+  //__local C c();
+}
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -150,15 +150,13 @@
 TEST(__local)
 
 // COMMON-LABEL: _Z11test__localv
-// EXPL: @__cxa_guard_acquire
 
-// Test the address space of 'this' when invoking a constructor for an object 
in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an object in local address space.
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking a method.
 // COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* 
addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C 
addrspace(4)*))
 
-
 // Test the address space of 'this' when invoking copy-constructor.
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C 
addrspace(4)*
 // EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], 
%class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)* 
@_ZZ11test__localvE1c to %class.C addrspace(4)*))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11645,7 +11645,11 @@
   setFunctionHasBranchProtectedScope();
   }
 }
-
+// In OpenCL, we can't initialize objects in the __local address space,
+// even implicitly, so don't synthesize an implicit initializer.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;
 // C++03 [dcl.init]p9:
 //   If no initializer is specified for an object, and the
 //   object is of (possibly cv-qualified) non-POD class type (or


Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+// Test that we don't initialize local address space objects.
+//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+class C {
+  int i;
+};
+
+kernel void test() {
+  __local int i;
+  __local C ii;
+  // FIXME: In OpenCL C we don't accept initializers for local
+  // address space variables. User defined initialization could
+  // make sense, but would it mean that all work items need to
+  // execute it? Potentially disallowing any initialization would
+  // make things easier and assingments can be used to set specific
+  // values. This rules should make it consistent with OpenCL C.
+  //__local C c();
+}
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -150,15 +150,13 @@
 TEST(__local)
 
 // COMMON-LABEL: _Z11test__localv
-// EXPL: @__cxa_guard_acquire
 
-// Test the address space of 'this' when invoking a constructor for an object in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-03-26 Thread John McCall via Phabricator via cfe-commits 
rjmccall added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11648-11651
+// In OpenCL we don't allow to initialize objects in local address space.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;

Anastasia wrote:
> bader wrote:
> > Shouldn't we invalidate Var declaration?
> This early exit is to prevent adding default initializer implicitly to the 
> declarations that are valid.
"In OpenCL, we can't initialize objects in the __local address space, even 
implicitly, so don't synthesize an implicit initializer."

I think it's important to add the underscores on `__local` to make it obvious 
that we're talking about OpenCL's "local" address space, not the address space 
that local variables appear in.


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

https://reviews.llvm.org/D59646



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-03-25 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia marked an inline comment as done.
Anastasia added inline comments.



Comment at: test/CodeGenOpenCLCXX/local_addrspace_init.cl:12
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space 
variables.

bader wrote:
> Anastasia wrote:
> > bader wrote:
> > > I guess this declaration should be disallowed for non-POD types. Can we 
> > > add a check for that to some test/Sema* test?
> > What should be disallowed specifically? Declaring non-POD types in the 
> > local address space?
> Something like 
> 
> ```
> class C {
>   int i = 0;
> };
> 
> kernel void test() {
>   __local C c; // error
> }
> ```
Yes, potentially we should disallow those or specify that the initialization is 
to be ignored. I plan to look into all of those. But for this change I am just 
fixing the issue reported in the bug and adding a FIXME for the future work.

Thanks!


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

https://reviews.llvm.org/D59646



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-03-25 Thread Alexey Bader via Phabricator via cfe-commits 
bader added inline comments.



Comment at: test/CodeGenOpenCLCXX/local_addrspace_init.cl:12
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space 
variables.

Anastasia wrote:
> bader wrote:
> > I guess this declaration should be disallowed for non-POD types. Can we add 
> > a check for that to some test/Sema* test?
> What should be disallowed specifically? Declaring non-POD types in the local 
> address space?
Something like 

```
class C {
  int i = 0;
};

kernel void test() {
  __local C c; // error
}
```


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

https://reviews.llvm.org/D59646



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-03-25 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11648-11651
+// In OpenCL we don't allow to initialize objects in local address space.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;

bader wrote:
> Shouldn't we invalidate Var declaration?
This early exit is to prevent adding default initializer implicitly to the 
declarations that are valid.



Comment at: test/CodeGenOpenCLCXX/local_addrspace_init.cl:12
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space 
variables.

bader wrote:
> I guess this declaration should be disallowed for non-POD types. Can we add a 
> check for that to some test/Sema* test?
What should be disallowed specifically? Declaring non-POD types in the local 
address space?


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

https://reviews.llvm.org/D59646



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-03-25 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia updated this revision to Diff 192087.
Anastasia marked 2 inline comments as done.
Anastasia added a comment.

Fixed typo in the comment.


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

https://reviews.llvm.org/D59646

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl


Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck 
%s
+
+// Test that we don't initialize local address space objects.
+//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+class C {
+  int i;
+};
+
+kernel void test() {
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space 
variables.
+  //User defined initialization could make sense, but would it mean that all 
work
+  //items need to execute it? Potentially disallowing any initialization would 
make
+  //things easier and assingments can be used to set specific values. This 
rules should
+  // make it consistent with OpenCL C.
+  //__local C c();
+}
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -150,15 +150,13 @@
 TEST(__local)
 
 // COMMON-LABEL: _Z11test__localv
-// EXPL: @__cxa_guard_acquire
 
-// Test the address space of 'this' when invoking a constructor for an object 
in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an object in local address space
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking a method.
 // COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* 
addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C 
addrspace(4)*))
 
-
 // Test the address space of 'this' when invoking copy-constructor.
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C 
addrspace(4)*
 // EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], 
%class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)* 
@_ZZ11test__localvE1c to %class.C addrspace(4)*))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11645,7 +11645,10 @@
   setFunctionHasBranchProtectedScope();
   }
 }
-
+// In OpenCL we don't allow to initialize objects in local address space.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;
 // C++03 [dcl.init]p9:
 //   If no initializer is specified for an object, and the
 //   object is of (possibly cv-qualified) non-POD class type (or


Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+// Test that we don't initialize local address space objects.
+//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+class C {
+  int i;
+};
+
+kernel void test() {
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space variables.
+  //User defined initialization could make sense, but would it mean that all work
+  //items need to execute it? Potentially disallowing any initialization would make
+  //things easier and assingments can be used to set specific values. This rules should
+  // make it consistent with OpenCL C.
+  //__local C c();
+}
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -150,15 +150,13 @@
 TEST(__local)
 
 // COMMON-LABEL: _Z11test__localv
-// EXPL: @__cxa_guard_acquire
 
-// Test the address space of 'this' when invoking a constructor for an object in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an object in local address space
+// EXPL-NOT: call void

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-03-24 Thread Alexey Bader via Phabricator via cfe-commits 
bader added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11648-11651
+// In OpenCL we don't allow to initialize objects in local address space.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;

Shouldn't we invalidate Var declaration?



Comment at: test/CodeGenOpenCLCXX/addrspace-of-this.cl:154
 
-// Test the address space of 'this' when invoking a constructor for an object 
in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an onbject in local address space
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))

onbject  -> object



Comment at: test/CodeGenOpenCLCXX/local_addrspace_init.cl:12
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space 
variables.

I guess this declaration should be disallowed for non-POD types. Can we add a 
check for that to some test/Sema* test?


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

https://reviews.llvm.org/D59646



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

2019-03-21 Thread Anastasia Stulova via Phabricator via cfe-commits 
Anastasia created this revision.
Anastasia added reviewers: rjmccall, bader.
Herald added a subscriber: ebevhan.
Anastasia retitled this revision from "[PR40778][PR41157] Prevent implicit 
initialization of local address space objects" to "[PR40778][PR41157][OpenCL] 
Prevent implicit initialization of local address space objects".
Herald added a subscriber: yaxunl.

In OpenCL C variables in local address space can't be initialized (s6.5.2). 
However in C++ default initialization can be performed even then there is no 
initializer explicitly provided. This won't work for local address space 
objects because they are not the same as regular global or local variables.

Current solution is to disable implicit default initialization. We still need 
some work to be done on user defined initialization and also construction of 
objects in local address space.


https://reviews.llvm.org/D59646

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl


Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck 
%s
+
+// Test that we don't initialize local address space objects.
+//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+class C {
+  int i;
+};
+
+kernel void test() {
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space 
variables.
+  //User defined initialization could make sense, but would it mean that all 
work
+  //items need to execute it? Potentially disallowing any initialization would 
make
+  //things easier and assingments can be used to set specific values. This 
rules should
+  // make it consistent with OpenCL C.
+  //__local C c();
+}
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -150,15 +150,13 @@
 TEST(__local)
 
 // COMMON-LABEL: _Z11test__localv
-// EXPL: @__cxa_guard_acquire
 
-// Test the address space of 'this' when invoking a constructor for an object 
in non-default address space
-// EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// Test that we don't initialize an onbject in local address space
+// EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast 
(%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking a method.
 // COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* 
addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C 
addrspace(4)*))
 
-
 // Test the address space of 'this' when invoking copy-constructor.
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C 
addrspace(4)*
 // EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], 
%class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)* 
@_ZZ11test__localvE1c to %class.C addrspace(4)*))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11645,7 +11645,10 @@
   setFunctionHasBranchProtectedScope();
   }
 }
-
+// In OpenCL we don't allow to initialize objects in local address space.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;
 // C++03 [dcl.init]p9:
 //   If no initializer is specified for an object, and the
 //   object is of (possibly cv-qualified) non-POD class type (or


Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+// Test that we don't initialize local address space objects.
+//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+class C {
+  int i;
+};
+
+kernel void test() {
+  __local int i;
+  __local C ii;
+  //FIXME: In OpenCL C we don't accept initializers for local address space variables.
+  //User defined initialization could make sense, but would it mean that all work
+  //items need to execute it? Potentially disallowing any initialization would make
+  //things easier and assingments can be used to set specific values. This rules should
+  // make it consistent with OpenCL C.
+  //__local C c();
+}
Index: