[OpenCL] Allow alloca return non-zero private pointer

[yaxunl]

Summary

There is an incoming change in LLVM allowing alloca to return a private pointer which does not pointing to address space 0:

https://reviews.llvm.org/D31042

After this change is committed, alloca will return a pointer pointing to an address space specified by the data layout (so called alloca addr space, which is the last component of the data layout, e.g. A5 indicating alloca address space is 5). A data layout not specifying alloca address space will assume it is 0, therefore keeping the original behaviour.

Clang codegen needs to make corresponding changes to account for the API change of alloca. The change is straightforward. Basically when creating alloca, use the alloca address space specified by the data layout.

For OpenCL, the private address space qualifier is 0 in AST. Before this change, 0 address space qualifier is always mapped to target address space 0. As now target private address space is specified by alloca address space in data layout, address space qualifier 0 needs to be mapped to alloca addr space specified by the data layout.

This change has no impact on targets whose alloca addr space is 0.

This change depends on

https://reviews.llvm.org/D31042

https://reviews.llvm.org/D31589

[t-tye]

lib/AST/ASTContext.cpp

9547–9555

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.

[yaxunl]

lib/AST/ASTContext.cpp

9547–9555

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.

[t-tye]

lib/AST/ASTContext.cpp

9547–9555

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.

[yaxunl]

lib/AST/ASTContext.cpp

9547–9555

Clang will emit error if address space value is negative, but I can change it to a warning.

[t-tye]

lib/AST/ASTContext.cpp

9547–9555

I guess the observation is that without a change the proposed changes will make attribute((address_space(0))) behave in unexpected ways for some targets (for AMDGPU it would actually cause address space for private to be used). The suggested approach also seems cleaner as it explicitly defines a "default" value which does not overlay the target address space range of values.

[yaxunl]

Revised by Tony's comments.

[t-tye]

include/clang/AST/ASTContext.h

2319

Should this be >= since it wants to return for all target address spaces, and target_first is the first one?

include/clang/AST/Type.h

336–342 ↗

(On Diff #93329)

Is this the right thing to do? This is making the CLANG named address spaces have the same numbers as the target-specific address space numbers which would seem rather confusing.

What do the diagnostics want to display? For example, they could display the address space as a human readable form such as "Default", "OpenCL-global", CUDA-device", "target-specific(5)", etc. To do that this function could take an iostream and a LangAS value and use << to write to the iostream.

[yaxunl]

include/clang/AST/ASTContext.h

2319

Will do.

include/clang/AST/Type.h

336–342 ↗

(On Diff #93329)

This function is used by diagnostics for address spaces specified by __attribute__((address_space(n))). There are several lit tests for that, e.g.

https://github.com/llvm-mirror/clang/blob/master/test/SemaCXX/address-space-newdelete.cpp
https://github.com/llvm-mirror/clang/blob/master/test/SemaCXX/address-space-references.cpp

It is desirable to use the same value as specified by the attribute, otherwise it may confuse the user.

[yaxunl]

include/clang/AST/Type.h

336–342 ↗

(On Diff #93329)

I can change it to

unsigned getAddressSpacePrintValue() const {
  return getAddressSpace() - LangAS::target_first;
}

Since the value is only used for __attribute__((address_space(n))), in case the user specifies negative value to achieve language specific addr space, the diag msg will just show the same negative value they used in __attribute__((address_space(n)))

[Anastasia]

I can't see clearly why the alloca has to be extended to accommodate the address space too? Couldn't the address space for alloca just be taken directly from the data layout?

In fact is seems from the LLVM review, an address space for alloca doesn't go into the bitcode.

include/clang/Basic/AddressSpaces.h

28 ↗

(On Diff #93329)

Somehow I wish that opencl_private would be represented explicitly instead and then an absence of an address space attribute would signify the default one to be used. But since opencl_private has always been represented as an absence of an address space attribute not only in AST but in IR as well, I believe it might be a bigger change now. However, how does this default address space align with default AS we put during type parsing in processTypeAttrs (https://reviews.llvm.org/D13168). I think after this step we shouldn't need default AS explicitly any longer?

41 ↗

(On Diff #93329)

I don't entirely understand the motivation for this. I think the idea of LangAS is to represent the source ASes while target ASes are reflected in the Map of Targets.cpp.

lib/AST/ASTContext.cpp

9553

Here it seems that LangAS::Default is indeed opencl_private?

test/Sema/invalid-assignment-constant-address-space.c

4 ↗

(On Diff #93329)

Is this test even correct? I don't think we can assume that C address spaces inherit the same restrictions as OpenCL. Especially that the notion of private/local/constant/global is an OpenCL specific thing.

I feel like Clang doesn't behave correctly for C address spaces now.

As for OpenCL I don't see why would anyone use attribute((address_space())) for constant AS. Especially that it's not part of the spec.

[yaxunl]

In D31404#714244, @Anastasia wrote:

I can't see clearly why the alloca has to be extended to accommodate the address space too? Couldn't the address space for alloca just be taken directly from the data layout?

In fact is seems from the LLVM review, an address space for alloca doesn't go into the bitcode.

In the latest comments of the LLVM review, reviewers have agreed that address space of alloca goes into the bitcode.

I am not quite get your first question. Do you mean why the API of alloca has to have an address space parameter? Or do you question the necessity to let alloca returning a pointer pointing to non-zero address space?

include/clang/Basic/AddressSpaces.h

28 ↗

(On Diff #93329)

Currently in Clang having an address space qualifier value of 0 is equivalent to having no address space qualifier. This is due to the internal representation of address space qualifier as bits in a mask. Therefore although there are separate API's for hasAddressSpace() and getAddressSpace(), in many many places people just do not use hasAddressSpace() and only use getAddressSpace(). In a way, this is convenient, since it allows people to use just one unsigned to represent that whether a type has an address space qualifier and the value of the qualifier if it has one. That's why value 0 of address space qualifier is called Default, since it indicates no address space qualifier, which is the default situation. Here we give it the name Default, just to emphasise the existing reality, that is, 0 is truely the default value of address space qualifier. This also matches most languages' view of address space, that is, if not explicitly specified, 0 is the default address space qualifier since it means no address space qualifier.

For OpenCL 1.2, this matches perfectly to private address space, since if no address space qualifier implies private address space. For OpenCL 2.0, things become complicated. 'no address space qualifier' in the source code no longer ends up with a fixed address space qualifier in AST. What address space qualifier we get in AST depends on scope of the variable. To be consistent with the AST of OpenCL 1.2, we continue to use 'no address space qualifier (or 0 value address space qualifier)' in AST to represent private address space in OpenCL source language. This is non-ideal but it works fine. Therefore although it is not written, in fact opencl_private is 0.

Since address space 0 in AST always represents the private address space in OpenCL and the default address space in other languages, it cannot be used for other address spaces of OpenCL. Also, when mapped to target address space, for OpenCL, address space 0 in AST should map to target private address space or alloca address space; for other languages, address space 0 in AST should map to target generic address space. It would be clearer to have an enum value for 0 instead of using 0 directly.

41 ↗

(On Diff #93329)

There are two types of address spaces in languages end up as address spaces in AST:

1. language defined address spaces, e.g. global in OpenCL => mapped to target address space
2. __attribute__((address_space(n))) => directly used as target address space with the same value

Since address space 0 in AST represents the default address space (no address space), it must be part of language address spaces and be mapped. Then it may be mapped to a target address space which is not 0.

Here is the problem: a user may use __attribute__((address_space(0))) to specify target address space 0, but he/she cannot, since address space 0 is always mapped as a language address space.

To solve this issue, address spaces from __attribute__((address_space(n))) is added to by Count when stored in AST. When mapped to target address space, their value is deducted by Count. Therefore, __attribute__((address_space(0))) becomes representable in AST.

lib/AST/ASTContext.cpp

9553

For OpenCL, that's true, however LangAS::Default may also be used by other languages to represent the default address space (i.e. no address space).

test/Sema/invalid-assignment-constant-address-space.c

4 ↗

(On Diff #93329)

I agree. There is no guarantee that in C language a user specified address space which happens to have the same address space value as OpenCL constant in AST will have the same semantics as OpenCL constant, because we only guarantee the semantics in OpenCL. For example, if we add a check for language for this diagnostic, this test will fail.

A user should not expect the same semantics. Only the target address space in the generated IR is guaranteed.

[Anastasia]

In D31404#714906, @yaxunl wrote:

In D31404#714244, @Anastasia wrote:

I can't see clearly why the alloca has to be extended to accommodate the address space too? Couldn't the address space for alloca just be taken directly from the data layout?

In fact is seems from the LLVM review, an address space for alloca doesn't go into the bitcode.

In the latest comments of the LLVM review, reviewers have agreed that address space of alloca goes into the bitcode.

I am not quite get your first question. Do you mean why the API of alloca has to have an address space parameter? Or do you question the necessity to let alloca returning a pointer pointing to non-zero address space?

I am asking mainly about the motivation of extending alloca instruction syntax with an extra address space. It seems to me that the address space for alloca could just be taken from the data layout without any modification to the instruction itself.

include/clang/Basic/AddressSpaces.h

28 ↗

(On Diff #93329)

I see a little value adding Default explicitly here, also because default AS is an OpenCL specific thing in my opinion. In C objects are just allowed to have no AS and it's fine. In fact the only use of Default in your patch is synonym to opencl_private. Unless we find another use for this, I would prefer not to add code for potential future use cases because it adds confusion and can be easily misused or create complications for adding new features.

I would rather say that Clang is missing an explicit representation of opencl_private at the moment. Mainly because private was used as default AS before OpenCL 2.0. So it was just the same thing. In OpenCL2.0 we worked around the fact that private is not represented explicitly by adding 'missing' ASes in processTypeAttrs. But it created some issues (for example in detecting NULL literal) which are still not solved.

Because ASes are represented as type qualifiers having opencl_private explicitly would align with C qualifiers concept (i.e. type with const would have a unique flag set to 1 in Qualifiers Mask and w/o const would have no flag set -> 0 ).

41 ↗

(On Diff #93329)

I was mainly asking about adding target_first.

I think general intension of this enum was to specify all existing explicitly known ASes to be able to identify them easily in the code instead of using hard coded numbers.

lib/AST/ASTContext.cpp

9553

Do we know what the other use cases could be?

test/Sema/invalid-assignment-constant-address-space.c

4 ↗

(On Diff #93329)

I think we should move this tests into CL and use __constant. Also it would be nice to add LangOpts.OpenCL check to where we give the error.

[yaxunl]

lib/AST/ASTContext.cpp

9553

It is used in the address space mapping to allow the default address space in AST to be mapped to target specified address space for non-OpenCL languages. For AMDGPU target, this maps to address space 4 for the non-amdgiz environment.

test/Sema/invalid-assignment-constant-address-space.c

4 ↗

(On Diff #93329)

Will do.

[yaxunl]

Update for the llvm IRBuilder API change for alloca. Removed data layout argument.
Moved a lit test to SemaOpenCL.

[t-tye]

include/clang/AST/Type.h

339–340 ↗

(On Diff #93820)

Since you mention this is only used for __attribute__((address_space(n))), why is this check for 0 needed?

If it is needed then to match other places should it simply be:

if (Addr)
  return Addr - LangAS::target_first;
return 0;

include/clang/Basic/DiagnosticSemaKinds.td

2451–2453 ↗

(On Diff #93820)

Now the one questionable test has been fixed, should the handling of address_space attribute go back to it being an error if n is negative? That seems more logical.

lib/AST/ASTContext.cpp

9555

To be consistent with other places should this simply be:

if (!AS && LangOpts.OpenCL)

9556

An alternative to doing this would be to add an opencl_private to LangAS and have each target map it accordingly. Then this could be:

// If a target specific address space was specified, simply return it.
if (AS >= LangAS::target_first)
  return AS - LangAS::target_first;
// For OpenCL, only function local variables are not explicitly marked with
// an address space in the AST, so treat them as the OpenCL private address space.
if (!AS && LangOpts.OpenCL)
  AS = LangAS::opencl_private;
return (*AddrSpaceMap)[AS];

This seems to better express what is happening here. If no address space was specified, and the language is OpenCL, then treat it as OpenCL private and map it according to the target mapping.

If wanted to eliminate the LangAS::Default named constant then that would be possible as it is no longer being used by name. However, would need to ensure that the first named enumerators starts at 1 so that 0 is left as the "no value explicitly specified" value that each target must map to the target specific generic address space.

lib/Sema/SemaExprCXX.cpp

2052 ↗

(On Diff #93820)

Should this be >= LangAS::target_first ? Seems it is wanting to check if an __attribute__((address_space(n))) was specified.

3123 ↗

(On Diff #93820)

Ditto.

[yaxunl]

Revised by Tony's comments.

[Anastasia]

include/clang/AST/ASTContext.h

2328

So we couldn't use the LangAS::Count instead?

I have the same comment in other places that use LangAS::target_first. Why couldn't we simply use LangAS::Count? It there any point in having two tags?

Another comment is why do we need ASes specified by __attribute__((address_space(n))) to be unique enum number at the end of named ASes of OpenCL and CUDA? I think conceptually the full range of ASes can be used in C because the ASes from OpenCL and CUDA are not available there anyways.

include/clang/AST/Type.h

339–340 ↗

(On Diff #93820)

Could we use LangAS::Count instead?

lib/AST/ASTContext.cpp

8732

Also here, could we use LangAS::Count instead?

9556

I would very much like to see opencl_private represented explicitly. This would allow us to simplify some parsing and also enable proper support of NULL literal (that has no AS by the spec).

We can of course do this refactoring work as a separate step.

lib/Sema/SemaType.cpp

5537 ↗

(On Diff #93820)

Do we need this temporary variable here?

[t-tye]

lib/Sema/SemaExprCXX.cpp

2055 ↗

(On Diff #93870)

Would suggest renaming getAddressSpacePrintValue to getAddressSpaceAttributePrintValue since it only deals with address spaces coming from the` attribute((address_space(n)))`.

[yaxunl]

include/clang/AST/ASTContext.h

2328

I will use LangAS::Count instead and remove target_first, since their values are the same.

For your second question: the values for __attribute__((address_space(n))) need to be different from the language specific address space values because they are mapped to target address space differently.

For language specific address space, they are mapped through a target defined mapping table.

For __attribute__((address_space(n))), the target address space should be the same as n, without going through the mapping table.

If they are defined in overlapping value ranges, they cannot be handled in different ways.

include/clang/AST/Type.h

339–340 ↗

(On Diff #93820)

It is for __attribute__((address_space(n))) and the default addr space 0.

For the default addr space 0, we want to print 0 instead of -LangAS::target_first.

I will make the change for matching other places.

include/clang/Basic/DiagnosticSemaKinds.td

2451–2453 ↗

(On Diff #93820)

Will do

lib/AST/ASTContext.cpp

9555

Will do.

9556

Introducing opencl_private could incur quite a few changes to AST, Sema, and lit tests.

We'd better do that in another patch since the main objective of this patch is to get Clang codegen work with the new alloca API and non-zero private address space.

lib/Sema/SemaExprCXX.cpp

2055 ↗

(On Diff #93870)

Will do

lib/Sema/SemaType.cpp

5537 ↗

(On Diff #93820)

will remove it.

[t-tye]

include/clang/AST/Type.h

339–340 ↗

(On Diff #93820)

I do not think the address space 0 should be returned as 0 as then it is impossible to distinguish between a type that has no address space attribute, and one that has an explicit address space attribute with the value 0.

But that seems to be a bug in the original code so I would suggest leaving this for now and fixing it as a separate patch. The diagnostic message should really be checking if an address space attribute was present (by checking for 0), and changing the working of the message accordingly.

Suggest add a TODO here to mention this which can be fixed in a later patch.

[yaxunl]

Revised by Tony's and Anastasia's comments.

[t-tye]

LGTM

[Anastasia]

include/clang/AST/ASTContext.h

2328

Target address space map currently corresponds to the named address spaces of OpenCL and CUDA only. So if the idea is to avoid overlapping with those we should extend the table? Although, I don't see how this can be done because it will require fixed semantic of address spaces in C which is currently undefined.

include/clang/AST/Type.h

345 ↗

(On Diff #93928)

ToDo -> TODO

lib/AST/ASTContext.cpp

9556

Makes sense!

lib/Sema/SemaExprCXX.cpp

3121 ↗

(On Diff #93928)

Pointee.getAddressSpace() wouldn't work any more?

[Anastasia]

include/clang/AST/ASTContext.h

2328

Perhaps I am missing something but I don't see any change here that makes non-named address spaces follow different path for the target.

Also does this change relate to alloca extension in some way? I still struggle to understand this fully...

All I can see is that this change restricts overlapping of named and non-named address spaces but I can't clearly understand the motivation for this.

lib/Basic/Targets.cpp

2035

This should use address space attribute 4 for all non-AS type objects. Could we make sure we have a codegen test for this?

[yaxunl]

include/clang/AST/ASTContext.h

2328

__attribute__((address_space(n))) is used in C and C++ to specify target address space for a variable.

For example, https://github.com/llvm-mirror/clang/blob/master/test/Sema/address_spaces.c

Many cases they just need to put a variable in certain target address space and do not need specific semantics for these address spaces.

2328

In the definition of ASTContext::getTargetAddressSpace() you can see how address space values below and above LangAS::Count are handled differently.

The old definition of address space enum does not differentiate between the default address space 0 (no address space qualifier) and __attribute__((address_space(0))).

Before alloca API changes, this does not matter, since address space 0 in AST is always mapped to target address space 0.

However, after alloca API changes, default address space 0 is mapped to target alloca address space, which may be non-zero. Therefore it is necessary to differentiate between __attribute__((address_space(0))) and the default address space 0. Otherwise, if a user specifies __attribute__((address_space(0))), it may not be mapped to target address space 0.

lib/Basic/Targets.cpp

2035

Will do.

lib/Sema/SemaExprCXX.cpp

3121 ↗

(On Diff #93928)

It still works. I will fix it.

[yaxunl]

Revised by Anastasia's comments.
Add lit test for __attribute__((address_space(0))) and default address space.

[yaxunl]

Ping! Any further questions? Thanks.

[Anastasia]

include/clang/AST/ASTContext.h

2328

However, after alloca API changes, default address space 0 is mapped to target alloca address space, which may be non-zero. Therefore it is necessary to differentiate between attribute((address_space(0))) and the default address space 0.

In your patch the default AS corresponds to value set in the target address space map and not taken from the alloca AS of the DataLayout.

include/clang/Basic/AddressSpaces.h

32 ↗

(On Diff #94102)

The alloca AS is not taken from the target AS map but from the DataLayout. This keep me wonder whether the explicit Default item is actually needed here....

lib/Basic/Targets.cpp

8382

This will break use case of compiling for SPIR from C which is needed by some frameworks.
We can only use generic if compiled in OpenCL2.0 mode and only for pointer types.

lib/Sema/SemaExprCXX.cpp

3121 ↗

(On Diff #93928)

Do you plan to update this?

test/CodeGen/default-address-space.c

51 ↗

(On Diff #94102)

The default AS is not the same as alloca AS then?

[yaxunl]

include/clang/AST/ASTContext.h

2328

Addr space 0 is mapped to alloca addr space for OpenCL and mapped to target-specified addr space for other languages, which is done by ASTContext::getTargetAddressSpace.

In either case, it may not be mapped to 0.

include/clang/Basic/AddressSpaces.h

32 ↗

(On Diff #94102)

For OpenCL, the default addr space is mapped to alloca addr space. For other languages, it is mapped by the address space mapping table.

lib/Basic/Targets.cpp

8382

Will change it to 0.

lib/Sema/SemaExprCXX.cpp

3121 ↗

(On Diff #93928)

Sorry I missed this. Will do.

test/CodeGen/default-address-space.c

51 ↗

(On Diff #94102)

The default AS is mapped to alloca AS only for OpenCL. For other languages, it is mapped by the mapping table.

[yaxunl]

Revised by Anastasia's comments.

[Anastasia]

include/clang/Basic/AddressSpaces.h

32 ↗

(On Diff #94102)

Ok. BTW, why is it done differently for other languages than OpenCL now? Is it something we missed in the programming model before? Or is it something specific to AMD target support?

[yaxunl]

include/clang/Basic/AddressSpaces.h

32 ↗

(On Diff #94102)

This is because amdgcn target now supports a new address space mapping where alloca address space is different from generic address space. For OpenCL, address space 0 should be mapped to alloca address space; for other languages, address space 0 should be mapped to a target specific generic address space. The previous approach for mapping address spaces cannot handle the requirements since it always maps address space 0 to target address space 0. Therefore it needs to be extended for more general cases.

[yaxunl]

Any further comments? Thanks.

[Anastasia]

LGTM! Thanks!

include/clang/AST/ASTContext.h

   unsigned getTargetAddressSpace(QualType T) const {
     return getTargetAddressSpace(T.getQualifiers());
   }
 
   unsigned getTargetAddressSpace(Qualifiers Q) const {
     return getTargetAddressSpace(Q.getAddressSpace());
   }
 
-  unsigned getTargetAddressSpace(unsigned AS) const {
-    if (AS < LangAS::Offset || AS >= LangAS::Offset + LangAS::Count)
-      return AS;
-    else
-      return (*AddrSpaceMap)[AS - LangAS::Offset];
-  }
+  unsigned getTargetAddressSpace(unsigned AS) const;
 
   /// Get target-dependent integer value for null pointer which is used for
   /// constant folding.
   uint64_t getTargetNullPointerValue(QualType QT) const;
 
   bool addressSpaceMapManglingFor(unsigned AS) const {
     return AddrSpaceMapMangling || 
            AS < LangAS::Offset || 
            AS >= LangAS::Offset + LangAS::Count;
   }
 
 private:
   // Helper for integer ordering
   unsigned getIntegerRank(const Type *T) const;
 
 public:
   //===--------------------------------------------------------------------===//
   //                    Type Compatibility Predicates
   //===--------------------------------------------------------------------===//
 
   /// Compatibility predicates used to check assignment expressions.
   bool typesAreCompatible(QualType T1, QualType T2, 
                           bool CompareUnqualified = false); // C99 6.2.7p1
 
   bool propertyTypesAreCompatible(QualType, QualType); 

lib/AST/ASTContext.cpp

       // qualified with an address space.
       char *End;
       unsigned AddrSpace = strtoul(Str, &End, 10);
       if (End != Str && AddrSpace != 0) {
         Type = Context.getAddrSpaceQualType(Type, AddrSpace);
         Str = End;
       }
       if (c == '*')
         Type = Context.getPointerType(Type);
       else
         Type = Context.getLValueReferenceType(Type);
       break;
     }
     // FIXME: There's no way to have a built-in with an rvalue ref arg.
     case 'C':
       Type = Type.withConst();
       break;

   if (QT->getUnqualifiedDesugaredType()->isNullPtrType())
     AS = 0;
   else
     AS = QT->getPointeeType().getAddressSpace();
 
   return getTargetInfo().getNullPointerValue(AS);
 }
 
+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
+    return (*AddrSpaceMap)[AS - LangAS::Offset];
+}
+
 // Explicitly instantiate this in case a Redeclarable<T> is used from a TU that
 // doesn't include ASTContext.h
 template
 clang::LazyGenerationalUpdatePtr<
     const Decl *, Decl *, &ExternalASTSource::CompleteRedeclChain>::ValueType
 clang::LazyGenerationalUpdatePtr<
     const Decl *, Decl *, &ExternalASTSource::CompleteRedeclChain>::makeValue(
         const clang::ASTContext &Ctx, Decl *Value);

lib/Basic/Targets.cpp

 const char *const NVPTXTargetInfo::GCCRegNames[] = {"r0"};
 
 ArrayRef<const char *> NVPTXTargetInfo::getGCCRegNames() const {
   return llvm::makeArrayRef(GCCRegNames);
 }
 
 static const LangAS::Map AMDGPUPrivateIsZeroMap = {
     1,  // opencl_global
     3,  // opencl_local
     2,  // opencl_constant
     4,  // opencl_generic
     1,  // cuda_device
     2,  // cuda_constant
     3   // cuda_shared
 };
 static const LangAS::Map AMDGPUGenericIsZeroMap = {
     1,  // opencl_global

 static const char *const DataLayoutStringSIPrivateIsZero =
   "e-p:32:32-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32"
   "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
   "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64";
 
 static const char *const DataLayoutStringSIGenericIsZero =
   "e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32"
   "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
-  "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64";
+  "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5";
 
 class AMDGPUTargetInfo final : public TargetInfo {
   static const Builtin::Info BuiltinInfo[];
   static const char * const GCCRegNames[];
 
   struct AddrSpace {
     unsigned Generic, Global, Local, Constant, Private;
     AddrSpace(bool IsGenericZero_ = false){

       hasFMAF = true;
       hasLDEXPF = true;
     }
     auto IsGenericZero = isGenericZero(Triple);
     resetDataLayout(getTriple().getArch() == llvm::Triple::amdgcn ?
                     (IsGenericZero ? DataLayoutStringSIGenericIsZero :
                         DataLayoutStringSIPrivateIsZero)
                     : DataLayoutStringR600);
+    assert(DataLayout->getAllocaAddrSpace() == AS.Private);
 
     AddrSpaceMap = IsGenericZero ? &AMDGPUGenericIsZeroMap :
         &AMDGPUPrivateIsZeroMap;
     UseAddrSpaceMapMangling = true;
   }
 
   uint64_t getPointerWidthV(unsigned AddrSpace) const override {
     if (GPU <= GK_CAYMAN)

   case CK_i486:
   case CK_i586:
   case CK_Pentium:
   case CK_i686:
   case CK_PentiumPro:
   case CK_Lakemont:
     break;
   case CK_PentiumMMX:
   case CK_Pentium2:

lib/CodeGen/CGBuiltin.cpp

   case Builtin::BI__builtin_alloca: {
     Value *Size = EmitScalarExpr(E->getArg(0));
     const TargetInfo &TI = getContext().getTargetInfo();
     // The alignment of the alloca should correspond to __BIGGEST_ALIGNMENT__.
     unsigned SuitableAlignmentInBytes =
         CGM.getContext()
             .toCharUnitsFromBits(TI.getSuitableAlign())
             .getQuantity();
-    AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
+    AllocaInst *AI = Builder.CreateAlloca(CGM.getDataLayout(),
+        Builder.getInt8Ty(), Size);
     AI->setAlignment(SuitableAlignmentInBytes);
     return RValue::get(AI);
   }
 
   case Builtin::BI__builtin_alloca_with_align: {
     Value *Size = EmitScalarExpr(E->getArg(0));
     Value *AlignmentInBitsValue = EmitScalarExpr(E->getArg(1));
     auto *AlignmentInBitsCI = cast<ConstantInt>(AlignmentInBitsValue);
     unsigned AlignmentInBits = AlignmentInBitsCI->getZExtValue();
     unsigned AlignmentInBytes =
         CGM.getContext().toCharUnitsFromBits(AlignmentInBits).getQuantity();
-    AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
+    AllocaInst *AI = Builder.CreateAlloca(CGM.getDataLayout(),
+        Builder.getInt8Ty(), Size);
     AI->setAlignment(AlignmentInBytes);
     return RValue::get(AI);
   }
 
   case Builtin::BIbzero:
   case Builtin::BI__builtin_bzero: {
     Address Dest = EmitPointerWithAlignment(E->getArg(0));
     Value *SizeVal = EmitScalarExpr(E->getArg(1));

lib/CodeGen/CGCall.cpp

   Address ArgMemory = Address::invalid();
   const llvm::StructLayout *ArgMemoryLayout = nullptr;
   if (llvm::StructType *ArgStruct = CallInfo.getArgStruct()) {
     ArgMemoryLayout = CGM.getDataLayout().getStructLayout(ArgStruct);
     llvm::Instruction *IP = CallArgs.getStackBase();
     llvm::AllocaInst *AI;
     if (IP) {
       IP = IP->getNextNode();
-      AI = new llvm::AllocaInst(ArgStruct, "argmem", IP);
+      AI = new llvm::AllocaInst(ArgStruct,
+          CGM.getDataLayout().getAllocaAddrSpace(), "argmem", IP);
     } else {
       AI = CreateTempAlloca(ArgStruct, "argmem");
     }
     auto Align = CallInfo.getArgStructAlignment();
     AI->setAlignment(Align.getQuantity());
     AI->setUsedWithInAlloca(true);
     assert(AI->isUsedWithInAlloca() && !AI->isStaticAlloca());
     ArgMemory = Address(AI, Align);

lib/CodeGen/CGDecl.cpp

 
     llvm::Value *elementCount;
     QualType elementType;
     std::tie(elementCount, elementType) = getVLASize(Ty);
 
     llvm::Type *llvmTy = ConvertTypeForMem(elementType);
 
     // Allocate memory for the array.
-    llvm::AllocaInst *vla = Builder.CreateAlloca(llvmTy, elementCount, "vla");
+    llvm::AllocaInst *vla = Builder.CreateAlloca(CGM.getDataLayout(),
+        llvmTy, elementCount, "vla");
     vla->setAlignment(alignment.getQuantity());
 
     address = Address(vla, alignment);
   }
 
   setAddrOfLocalVar(&D, address);
   emission.Addr = address;
 

lib/CodeGen/CGExpr.cpp

Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
   Alloca->setAlignment(Align.getQuantity());
   return Address(Alloca, Align);
 }
 
 /// CreateTempAlloca - This creates a alloca and inserts it into the entry
 /// block.
 llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
                                                     const Twine &Name) {
-  return new llvm::AllocaInst(Ty, nullptr, Name, AllocaInsertPt);
+  return new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
+      nullptr, Name, AllocaInsertPt);
 }
 
 /// CreateDefaultAlignTempAlloca - This creates an alloca with the
 /// default alignment of the corresponding LLVM type, which is *not*
 /// guaranteed to be related in any way to the expected alignment of
 /// an AST type that might have been lowered to Ty.
 Address CodeGenFunction::CreateDefaultAlignTempAlloca(llvm::Type *Ty,
                                                       const Twine &Name) {

lib/CodeGen/CGObjCGNU.cpp

   // Load the superclass pointer
   ReceiverClass =
     Builder.CreateAlignedLoad(ReceiverClass, CGF.getPointerAlign());
   // Construct the structure used to look up the IMP
   llvm::StructType *ObjCSuperTy = llvm::StructType::get(
       Receiver->getType(), IdTy, nullptr);
 
   // FIXME: Is this really supposed to be a dynamic alloca?
-  Address ObjCSuper = Address(Builder.CreateAlloca(ObjCSuperTy),
-                              CGF.getPointerAlign());
+  Address ObjCSuper = Address(Builder.CreateAlloca(CGM.getDataLayout(),
+      ObjCSuperTy), CGF.getPointerAlign());
 
   Builder.CreateStore(Receiver,
                    Builder.CreateStructGEP(ObjCSuper, 0, CharUnits::Zero()));
   Builder.CreateStore(ReceiverClass,
                    Builder.CreateStructGEP(ObjCSuper, 1, CGF.getPointerSize()));
 
   ObjCSuper = EnforceType(Builder, ObjCSuper, PtrToObjCSuperTy);
 

test/CodeGenOpenCL/address-spaces.cl

-// RUN: %clang_cc1 %s -O0 -ffake-address-space-map -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 %s -O0 -DCL20 -cl-std=CL2.0 -ffake-address-space-map -emit-llvm -o - | FileCheck %s --check-prefix=CL20
+// RUN: %clang_cc1 %s -O0 -ffake-address-space-map -emit-llvm -o - | FileCheck %s --check-prefixes=CHECK,SPIR
+// RUN: %clang_cc1 %s -O0 -DCL20 -cl-std=CL2.0 -ffake-address-space-map -emit-llvm -o - | FileCheck %s --check-prefixes=CL20,CL20SPIR
+// RUN: %clang_cc1 %s -O0 -triple amdgcn-amd-amdhsa-opencl -emit-llvm -o - | FileCheck --check-prefixes=CHECK,SPIR %s
+// RUN: %clang_cc1 %s -O0 -triple amdgcn-amd-amdhsa-opencl -DCL20 -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s --check-prefixes=CL20,CL20SPIR
+// RUN: %clang_cc1 %s -O0 -triple amdgcn-amd-amdhsa-amdgizcl -emit-llvm -o - | FileCheck %s -check-prefixes=CHECK,GIZ
+// RUN: %clang_cc1 %s -O0 -triple amdgcn-amd-amdhsa-amdgizcl -DCL20 -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s --check-prefixes=CL20,CL20GIZ
 
-// CHECK: i32* %arg
+// SPIR: i32* %arg
+// GIZ: i32 addrspace(5)* %arg
 void f__p(__private int *arg) {}
 
 // CHECK: i32 addrspace(1)* %arg
 void f__g(__global int *arg) {}
 
 // CHECK: i32 addrspace(3)* %arg
 void f__l(__local int *arg) {}
 
-// CHECK: i32 addrspace(2)* %arg
+// SPIR: i32 addrspace(2)* %arg
+// GIZ: i32 addrspace(4)* %arg
 void f__c(__constant int *arg) {}
 
-// CHECK: i32* %arg
+// SPIR: i32* %arg
+// GIZ: i32 addrspace(5)* %arg
 void fp(private int *arg) {}
 
 // CHECK: i32 addrspace(1)* %arg
 void fg(global int *arg) {}
 
 // CHECK: i32 addrspace(3)* %arg
 void fl(local int *arg) {}
 
-// CHECK: i32 addrspace(2)* %arg
+// SPIR: i32 addrspace(2)* %arg
+// GIZ: i32 addrspace(4)* %arg
 void fc(constant int *arg) {}
 
 #ifdef CL20
 int i;
 // CL20-DAG: @i = common addrspace(1) global i32 0
 int *ptr;
-// CL20-DAG: @ptr = common addrspace(1) global i32 addrspace(4)* null
+// CL20SPIR-DAG: @ptr = common addrspace(1) global i32 addrspace(4)* null
+// CL20GIZ-DAG: @ptr = common addrspace(1) global i32* null
 #endif
 
-// CHECK: i32* %arg
-// CL20-DAG: i32 addrspace(4)* %arg
+// SPIR: i32* %arg
+// GIZ: i32 addrspace(5)* %arg
+// CL20SPIR-DAG: i32 addrspace(4)* %arg
+// CL20GIZ-DAG: i32* %arg
 void f(int *arg) {
 
   int i;
-// CHECK: %i = alloca i32,
-// CL20-DAG: %i = alloca i32,
+// SPIR: %i = alloca i32,
+// GIZ: %i = alloca i32{{.*}}addrspace(5)
+// CL20SPIR-DAG: %i = alloca i32,
+// CL20GIZ-DAG: %i = alloca i32{{.*}}addrspace(5)
 
 #ifdef CL20
   static int ii;
 // CL20-DAG: @f.ii = internal addrspace(1) global i32 0
 #endif
 }

test/CodeGenOpenCL/amdgpu-env-amdgiz.cl

 // RUN: %clang_cc1 %s -O0 -triple amdgcn -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s -O0 -triple amdgcn---opencl -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s -O0 -triple amdgcn---amdgiz -emit-llvm -o - | FileCheck -check-prefix=GIZ %s
 // RUN: %clang_cc1 %s -O0 -triple amdgcn---amdgizcl -emit-llvm -o - | FileCheck -check-prefix=GIZ %s
 
 // CHECK: target datalayout = "e-p:32:32-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64"
-// GIZ: target datalayout = "e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64"
+// GIZ: target datalayout = "e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
 void foo(void) {}
 

test/CodeGenOpenCL/vla.cl

-// RUN: %clang_cc1 -emit-llvm -triple "spir-unknown-unknown" -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple "spir-unknown-unknown" -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,SPIR %s
+// RUN: %clang_cc1 -emit-llvm -triple amdgcn-amd-amdhsa-opencl -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,SPIR %s
+// RUN: %clang_cc1 -emit-llvm -triple amdgcn-amd-amdhsa-amdgizcl -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,GIZ %s
 
 constant int sz0 = 5;
-// CHECK: @sz0 = addrspace(2) constant i32 5
+// SPIR: @sz0 = addrspace(2) constant i32 5
+// GIZ: @sz0 = addrspace(4) constant i32 5
 const global int sz1 = 16;
 // CHECK: @sz1 = addrspace(1) constant i32 16
 const constant int sz2 = 8;
-// CHECK: @sz2 = addrspace(2) constant i32 8
+// SPIR: @sz2 = addrspace(2) constant i32 8
+// GIZ: @sz2 = addrspace(4) constant i32 8
 // CHECK: @testvla.vla2 = internal addrspace(3) global [8 x i16] undef
 
 kernel void testvla()
 {
   int vla0[sz0];
-// CHECK: %vla0 = alloca [5 x i32]
+// SPIR: %vla0 = alloca [5 x i32]
+// SPIR-NOT: %vla0 = alloca [5 x i32]{{.*}}addrspace
+// GIZ: %vla0 = alloca [5 x i32]{{.*}}addrspace(5)
   char vla1[sz1];
-// CHECK: %vla1 = alloca [16 x i8]
+// SPIR: %vla1 = alloca [16 x i8]
+// SPIR-NOT: %vla1 = alloca [16 x i8]{{.*}}addrspace
+// GIZ: %vla1 = alloca [16 x i8]{{.*}}addrspace(5)
   local short vla2[sz2];
 }