[PATCH] D31404: [OpenCL] Allow alloca return non-zero private pointer

[Tony Tye via Phabricator via cfe-commits]

t-tye 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
----------------
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.


https://reviews.llvm.org/D31404