This is an archive of the discontinued LLVM Phabricator instance.

[PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects
ClosedPublic

Authored by Anastasia on Mar 21 2019, 8:19 AM.

Details

Summary

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.

Diff Detail

Repository
rC Clang

Event Timeline

Anastasia created this revision.Mar 21 2019, 8:19 AM
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.Mar 21 2019, 8:19 AM
bader added inline comments.Mar 24 2019, 1:37 PM
lib/Sema/SemaDecl.cpp
11685–11690

Shouldn't we invalidate Var declaration?

test/CodeGenOpenCLCXX/addrspace-of-this.cl
154–155

onbject -> object

test/CodeGenOpenCLCXX/local_addrspace_init.cl
13

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

Anastasia updated this revision to Diff 192087.Mar 25 2019, 5:38 AM
Anastasia marked 2 inline comments as done.

Fixed typo in the comment.

lib/Sema/SemaDecl.cpp
11685–11690

This early exit is to prevent adding default initializer implicitly to the declarations that are valid.

test/CodeGenOpenCLCXX/local_addrspace_init.cl
13

What should be disallowed specifically? Declaring non-POD types in the local address space?

bader added inline comments.Mar 25 2019, 5:49 AM
test/CodeGenOpenCLCXX/local_addrspace_init.cl
13

Something like

class C {
  int i = 0;
};

kernel void test() {
  __local C c; // error
}
Anastasia marked an inline comment as done.Mar 25 2019, 7:26 AM
Anastasia added inline comments.
test/CodeGenOpenCLCXX/local_addrspace_init.cl
13

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!

rjmccall added inline comments.Mar 26 2019, 12:25 PM
lib/Sema/SemaDecl.cpp
11685–11690

"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.

Anastasia updated this revision to Diff 193474.Apr 3 2019, 5:32 AM

Improved comment about initializers in __local addr space.

rjmccall accepted this revision.Apr 3 2019, 6:59 AM

LGTM.

This revision is now accepted and ready to land.Apr 3 2019, 6:59 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2019, 4:07 AM