[PATCH] OpenCL: Emit global variables in the constant addr space as constant globals

[Tom Stellard]

On Fri, Oct 03, 2014 at 11:35:00AM -0700, Tom Stellard wrote:
> On Fri, Oct 03, 2014 at 11:23:24AM -0700, Tom Stellard wrote:
> > On Wed, Oct 01, 2014 at 10:30:19AM +0300, Pekka Jääskeläinen wrote:
> > > Hi Tom,
> > > 
> > > This patch seemed familiar to me, and by googling I found this
> > > patch for the constant string literals:
> > > 
> > > http://reviews.llvm.org/D894
> > > 
> > > "I updated this patch to remove the 'const' part as I now believe it
> > > is wrong, and I changed IsModifiable as Tanya suggested.", writes
> > > Joey,
> > > referring to this comment from Tanya:
> > > http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20130603/081103.html
> > > 
> > > The comment might apply to this patch as well. Perhaps adding a
> > > test for const __constant is a good idea?
> > > 
> > > As you didn't change this test:
> > > 
> > > > +++ test/SemaOpenCL/address-spaces.cl
> > > > @@ -9,5 +9,5 @@
> > > >   int *ip;
> > > >   ip = gip; // expected-error {{assigning '__global int *' to 'int
> > > *' changes address space of pointer}}
> > > >   ip = &li; // expected-error {{assigning '__local int *' to 'int
> > > *' changes address space of pointer}}
> > > > -  ip = &ci; // expected-error {{assigning '__constant int *' to
> > > 'int *' changes address space of pointer}}
> > > > +  ip = &ci; // expected-error {{assigning 'const __constant int
> > > *' to 'int *' changes address space of pointer}}
> > > 
> > > I assume the other part of the comment (confusing error message)
> > > is not a problem anymore.
> > 
> > Hi Pekka,
> > 
> > 
> > From what I can tell this patch is slightly different from the
> > other one, because instead of actually adding the const modifier
> > to __constant address space types, it updates the query that determines
> > whether or not a type is constant.  So there shouldn't be any issues
> > with mixing const and __constant.
> > 
> > Here is an updated patch with a test case for const __constant,
> > though it's not quite the same as the one you posted above.
> > In the thread Eli mentioned that const __constant was illegal in OpenCL
> > C, I couldn't find where in the spec where it says this.  It would
> > be good to get clarification on this, but I think that fix is unrelated
> > and maybe the new test case is too.
> > 
> > What do you think?
> > 
> 
> Forgot the patch...

Hi Pekka,

I understand now why you recommended the const __constant test cases.  I have
added these to the patch.  I also removed an unnecessary include I added to
Type.cpp

-Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-OpenCL-Emit-global-variables-in-the-constant-addr-sp.patch
Type: text/x-diff
Size: 6797 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141003/f3914b83/attachment.patch>