[PATCH] D39129: [OpenCL] Fix generation of constant address space sampler in function scope

2017-11-14 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

In https://reviews.llvm.org/D39129#924425, @bader wrote:

> In https://reviews.llvm.org/D39129#923235, @Anastasia wrote:
>
> > In https://reviews.llvm.org/D39129#902848, @bader wrote:
> >
> > > @Anastasia, during the discussion of similar fix 
> > > (https://reviews.llvm.org/D34342).
> > >
> > > I found another bug in the CodeGen library. Do you have time to fix it 
> > > too?
> > >  Here is the reproducer from the old review request:
> > >
> > >   int get_sampler_initializer(void);
> > >   kernel void foo() {
> > > const sampler_t const_smp_func_init = get_sampler_initializer();
> > >   }
> > >
> >
> >
> > I am getting an error currently: `error: internal error: could not emit 
> > constant value "abstractly"`. It seems very weird and also doesn't seem to 
> > be related to constant AS change.  I think this should be allowed to 
> > compile though? I can try to investigate but since I don't know the time 
> > frame yet and it doesn't seem to be related to constant AS I would prefer 
> > to do it as a separate change.
>
>
> I'm fine with fixing this test case later, but I assume that patch posted 
> https://reviews.llvm.org/D34342 covers more cases. Is this version able to 
> handle samples declared as `const sampler_t const_smp`?
>  Are you okay if I commit https://reviews.llvm.org/D34342 instead?


Sure. Sorry I overlooked it somehow.


https://reviews.llvm.org/D39129



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


[PATCH] D39129: [OpenCL] Fix generation of constant address space sampler in function scope

2017-11-14 Thread Alexey Bader via Phabricator via cfe-commits
bader added a comment.

In https://reviews.llvm.org/D39129#923235, @Anastasia wrote:

> In https://reviews.llvm.org/D39129#902848, @bader wrote:
>
> > @Anastasia, during the discussion of similar fix 
> > (https://reviews.llvm.org/D34342).
> >
> > I found another bug in the CodeGen library. Do you have time to fix it too?
> >  Here is the reproducer from the old review request:
> >
> >   int get_sampler_initializer(void);
> >   kernel void foo() {
> > const sampler_t const_smp_func_init = get_sampler_initializer();
> >   }
> >
>
>
> I am getting an error currently: `error: internal error: could not emit 
> constant value "abstractly"`. It seems very weird and also doesn't seem to be 
> related to constant AS change.  I think this should be allowed to compile 
> though? I can try to investigate but since I don't know the time frame yet 
> and it doesn't seem to be related to constant AS I would prefer to do it as a 
> separate change.


I'm fine with fixing this test case later, but I assume that patch posted 
https://reviews.llvm.org/D34342 covers more cases. Is this version able to 
handle samples declared as `const sampler_t const_smp`?
Are you okay if I commit https://reviews.llvm.org/D34342 instead?


https://reviews.llvm.org/D39129



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


[PATCH] D39129: [OpenCL] Fix generation of constant address space sampler in function scope

2017-11-13 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added a comment.

In https://reviews.llvm.org/D39129#902848, @bader wrote:

> @Anastasia, during the discussion of similar fix 
> (https://reviews.llvm.org/D34342).
>
> I found another bug in the CodeGen library. Do you have time to fix it too?
>  Here is the reproducer from the old review request:
>
>   int get_sampler_initializer(void);
>   kernel void foo() {
> const sampler_t const_smp_func_init = get_sampler_initializer();
>   }
>


I am getting an error currently: `error: internal error: could not emit 
constant value "abstractly"`. It seems very weird and also doesn't seem to be 
related to constant AS change.  I think this should be allowed to compile 
though? I can try to investigate but since I don't know the time frame yet and 
it doesn't seem to be related to constant AS I would prefer to do it as a 
separate change.


https://reviews.llvm.org/D39129



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


[PATCH] D39129: [OpenCL] Fix generation of constant address space sampler in function scope

2017-10-21 Thread Alexey Bader via Phabricator via cfe-commits
bader added a comment.

@Anastasia, during the discussion of similar fix 
(https://reviews.llvm.org/D34342).

I found another bug in the CodeGen library. Do you have time to fix it too?
Here is the reproducer from the old review request:

  int get_sampler_initializer(void);
  kernel void foo() {
const sampler_t const_smp_func_init = get_sampler_initializer();
  }


https://reviews.llvm.org/D39129



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


[PATCH] D39129: [OpenCL] Fix generation of constant address space sampler in function scope

2017-10-20 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia created this revision.

After the change of constant address space function scope variable in 6e34f0e 
("[OpenCL] Emit function-scope variable in constant address space as static 
variable", 2017-05-15)  a bug has been introduced that triggered unreachable 
during generation of samplers.

This is because sampler in global scope has not to be generated. The 
initialization function has to be used instead of a variable everywhere the 
sampler variable is being referenced.

This patch fixes the bug and adds missing test case.


https://reviews.llvm.org/D39129

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenOpenCL/sampler.cl


Index: test/CodeGenOpenCL/sampler.cl
===
--- test/CodeGenOpenCL/sampler.cl
+++ test/CodeGenOpenCL/sampler.cl
@@ -33,6 +33,10 @@
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 19)
   // CHECK: store %opencl.sampler_t addrspace(2)* [[SAMP]], %opencl.sampler_t 
addrspace(2)** [[smp_ptr]]
 
+  // Initialising constant AS sampler will be handled as global (we won't 
generate local alloca for it)
+  // CHECK-NOT: alloca %opencl.sampler_t addrspace(2)*
+  constant sampler_t smp_const_as = 11;
+
   // Case 1b
   fnc4smp(smp);
   // CHECK-NOT: call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 19)
@@ -58,4 +62,8 @@
   fnc4smp(5);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 5)
   // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
+
+  fnc4smp(smp_const_as);
+  // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* 
@__translate_sampler_initializer(i32 11)
+  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* 
[[SAMP]])
 }
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -158,6 +158,13 @@
 // Don't emit it now, allow it to be emitted lazily on its first use.
 return;
 
+   // Samplers in constant address space are handled as a special case
+   // unlike other variables they are not generated as globals
+   // and their initialiser will be used everywhere it is being referenced.
+   if (D.getType().getAddressSpace() == LangAS::opencl_constant &&
+   D.getType().getTypePtr()->isSamplerT())
+ return;
+
   // Some function-scope variable does not have static storage but still
   // needs to be emitted like a static variable, e.g. a function-scope
   // variable in constant address space in OpenCL.


Index: test/CodeGenOpenCL/sampler.cl
===
--- test/CodeGenOpenCL/sampler.cl
+++ test/CodeGenOpenCL/sampler.cl
@@ -33,6 +33,10 @@
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 19)
   // CHECK: store %opencl.sampler_t addrspace(2)* [[SAMP]], %opencl.sampler_t addrspace(2)** [[smp_ptr]]
 
+  // Initialising constant AS sampler will be handled as global (we won't generate local alloca for it)
+  // CHECK-NOT: alloca %opencl.sampler_t addrspace(2)*
+  constant sampler_t smp_const_as = 11;
+
   // Case 1b
   fnc4smp(smp);
   // CHECK-NOT: call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 19)
@@ -58,4 +62,8 @@
   fnc4smp(5);
   // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 5)
   // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
+
+  fnc4smp(smp_const_as);
+  // CHECK: [[SAMP:%[0-9]+]] = call %opencl.sampler_t addrspace(2)* @__translate_sampler_initializer(i32 11)
+  // CHECK: call spir_func void @fnc4smp(%opencl.sampler_t addrspace(2)* [[SAMP]])
 }
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -158,6 +158,13 @@
 // Don't emit it now, allow it to be emitted lazily on its first use.
 return;
 
+   // Samplers in constant address space are handled as a special case
+   // unlike other variables they are not generated as globals
+   // and their initialiser will be used everywhere it is being referenced.
+   if (D.getType().getAddressSpace() == LangAS::opencl_constant &&
+   D.getType().getTypePtr()->isSamplerT())
+ return;
+
   // Some function-scope variable does not have static storage but still
   // needs to be emitted like a static variable, e.g. a function-scope
   // variable in constant address space in OpenCL.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits