yaxunl added a comment.
In https://reviews.llvm.org/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?
================
Comment at: include/clang/Basic/AddressSpaces.h:28
enum ID {
- Offset = 0x7FFF00,
+ Default = 0,
----------------
Anastasia wrote:
> 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?
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.
================
Comment at: include/clang/Basic/AddressSpaces.h:41
+
+ target_first = Count
};
----------------
Anastasia wrote:
> 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.
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.
================
Comment at: lib/AST/ASTContext.cpp:9553
+ // alloca.
+ if (AS == LangAS::Default && LangOpts.OpenCL)
+ return getTargetInfo().getDataLayout().getAllocaAddrSpace();
----------------
Anastasia wrote:
> Here it seems that LangAS::Default is indeed opencl_private?
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).
================
Comment at: test/Sema/invalid-assignment-constant-address-space.c:4
-#define OPENCL_CONSTANT 8388354
-int __attribute__((address_space(OPENCL_CONSTANT))) c[3] = {0};
----------------
Anastasia wrote:
> 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.
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.
https://reviews.llvm.org/D31404
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
