yaxunl added inline comments.
================
Comment at: lib/AST/ASTContext.cpp:9547-9555
+unsigned ASTContext::getTargetAddressSpace(unsigned AS) const {
+ // For OpenCL, the address space qualifier is 0 in AST.
+ if (AS == 0 && LangOpts.OpenCL)
+ return getTargetInfo().getDataLayout().getAllocaAddrSpace();
+ if (AS < LangAS::Offset || AS >= LangAS::Offset + LangAS::Count)
+ return AS;
+ else
----------------
t-tye wrote:
> yaxunl wrote:
> > t-tye wrote:
> > > Presumably this will not work if the user put an address space attribute
> > > on a variable with an address space of 0, since it is not possible to
> > > distinguish between a variable that never had an attribute and was
> > > function local, and one that has an explicit address space attribute
> > > specifying 0.
> > >
> > > It would seem better if LangAS did not map the 0..LangAS::Offset to be
> > > target address spaces. Instead LangAS could use 0..LangAS::Count to be
> > > the CLANG explicitly specified values (reserving 0 to mean the default
> > > when none was specified), and values above LangAS::Count would map to the
> > > explicitly specified target address spaces. For example:
> > >
> > > ```
> > > namespace clang {
> > >
> > > namespace LangAS {
> > >
> > > /// \brief Defines the set of possible language-specific address spaces.
> > > ///
> > > /// This uses values above the language-specific address spaces to denote
> > > /// the target-specific address spaces biased by target_first.
> > > enum ID {
> > > default = 0,
> > >
> > > opencl_global,
> > > opencl_local,
> > > opencl_constant,
> > > opencl_generic,
> > >
> > > cuda_device,
> > > cuda_constant,
> > > cuda_shared,
> > >
> > > Count,
> > >
> > > target_first = Count
> > > };
> > >
> > > /// The type of a lookup table which maps from language-specific address
> > > spaces
> > > /// to target-specific ones.
> > > typedef unsigned Map[Count];
> > >
> > > }
> > > ```
> > >
> > > Then this function would be:
> > >
> > > ```
> > > unsigned ASTContext::getTargetAddressSpace(unsigned AS) const {
> > > if (AS == LangAS::default && LangOpts.OpenCL)
> > > // For OpenCL, only function local variables are not explicitly
> > > marked with an
> > > // address space in the AST, and these need to be the address space
> > > of alloca.
> > > return getTargetInfo().getDataLayout().getAllocaAddrSpace();
> > > if (AS >= LangAS::target_first)
> > > return AS - LangAS::target_first;
> > > else
> > > return (*AddrSpaceMap)[AS];
> > > }
> > > ```
> > >
> > > Each target AddrSpaceMap would map LangAS::default to that target's
> > > default generic address space since that matches most other languages.
> > >
> > > The address space attribute would need a corresponding "+
> > > LangAS::target_first" to the value it stored in the AST.
> > >
> > > Then it is possible to definitively tell when an AST node has not had any
> > > address space specified as it will be the LangAS::default value.
> > There is a lit test like this:
> >
> > ```
> > // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
> >
> > #define OPENCL_CONSTANT 8388354
> > int __attribute__((address_space(OPENCL_CONSTANT))) c[3] = {0};
> >
> > void foo() {
> > c[0] = 1; //expected-error{{read-only variable is not assignable}}
> > }
> >
> > ```
> > It tries to set address space of opencl_constant through
> > `__attribute__((address_space(n)))`. If we "+ LangAS::target_first" to the
> > value before it is tored in the AST, we are not able to use
> > `__attribute__((address_space(n)))` to represent opencl_constant.
> This seems a bit of a hack. It could be made to work by simply defining
> OPENCL_CONSTANT to be the negative value that would result in the correct
> LangAS value, which is pretty much what the test is doing anyway. Just seems
> conflating the default value with the first target address space value is
> undesirable as it prevents specifying target address space 0 as that gets
> treated differently than any other address space value.
Clang will emit error if address space value is negative, but I can change it
to a warning.
https://reviews.llvm.org/D31404
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
