[cfe-dev] [LLVMdev] Language-specific vs target-specific address spaces (was Re: [PATCH] OpenCL support - update on keywords)

[Ken Dyck]

On Tue, Mar 15, 2011 at 11:28 AM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> On Tue, Mar 15, 2011 at 07:40:58AM -0400, Ken Dyck wrote:
>> On Fri, Mar 11, 2011 at 4:59 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
>> > I have completed my patch (attached) based on my suggestions here
>> > and the subsequent discussion.
>>
>> Could the language-specific address space numbers be translated to
>> target-specific ones when the keywords are parsed in
>> Parser::ParseOpenCLAttributes()? Then the Qualifier would be
>> guaranteed to contain a target-specific address space and there
>> wouldn't really be any need for ASTContext::getTargetAddressSpace().
>> I'm a bit concerned that having two ways to get the address space of a
>> pointer type, each potentially giving different results, will cause
>> confusion and errors in the future.
>
> There are three main reasons for storing the language address space
> in the qualifier.
>
> The first is that on architectures where __constant, __local, and
> __global share an address space on the target (e.g. CPU) we still
> require pointers to the various address spaces to be incompatible in
> order to conform with section 6.5 of the OpenCL spec:
>  "A pointer to address space A can only be assigned to a pointer to
>   the same address space A. Casting a pointer to address space
>   A to a pointer to address space B is illegal."
>
> The second is that on some architectures, some address spaces will
> require special handling when dereferenced.  For example, on a CPU, a
> __local pointer dereference may involve adding a thread-local offset.
> In this case, we will need to preserve the distinction between the
> address spaces until LLVM generation time.
>
> The third reason is that tools which operate on OpenCL code using the
> AST (e.g. consider a source to source translator) may need access to
> the originally specified address space.

Fair enough. Then I'm fine with the patch.

-Ken