[cfe-dev] [RFC] Re-use OpenCL address space attributes for SYCL

[John McCall via cfe-dev]

On 6 Aug 2020, at 15:11, Anastasia Stulova wrote:
> Just to make it clear - C++ libraries or any existing C++ code doesn't
> necessarily stop to compile if address space attributes are being 
> used. It only
> fails on illegal use of address spaces defined by Embedded C spec from 
> where
> the implementation originates.

Alexey is correct that C and C++ require that if a variable `x` has type
`T`, the expression `&x` must have type `T *`.  Anything else is not
strictly conformant, whether it compiles or not.  OpenCL is therefore
not strictly conformant to the C/C++ specification, and that’s fine
because OpenCL is an independent language, not just a language 
extension;
it happens to be defined using C/C++ as a base specification, but its
modifications are always authoritative.  But as I understand it, Alexey
just wants a conformant C++ implementation on a different target, and
so needs to follow the rules.

It’s good that libc++ is broadly permissive about types qualified
with address spaces, but that doesn’t really change anything.

John.

>
> For example, we have made an evaluation by running type trait tests 
> from libcxx
> with a standard header in OpenCL mode and only a few tests failed due 
> to
> the address spaces. None of those required changes in the libcxx code. 
> Instead,
> they have revealed bugs in clang (that were fixed in release 11 btw) 
> and issues in
> tests due to illegal behavior. These issues are expected for the 
> address space
> attributes as there are extra restriction and rules on types 
> attributed by the
> address spaces that come from Embedded C. If these restrictions are 
> not desirable
> I feel the desirable behavior might be significantly different from 
> the intended use
> of address space attributes and therefore it might be better to look 
> at alternative
> approaches including adding a dedicated attribute that doesn't 
> propagate into a type
> qualifier at all.
>
>> casting a names address space pointer to "generic" pointer is valid 
>> operation
> in all GPGPU programming models I'm aware of (including OpenCL)
>
> I would like to highlight that generic address space and address space
> conversions have only been added in OpenCL 2.0. Furthermore, generic 
> address
> space is becoming optional functionality in OpenCL 3.0.
>
> Cheers,
> Anastasia
>
> ________________________________
> From: Bader, Alexey <alexey.bader at intel.com>
> Sent: 30 July 2020 20:51
> To: David Rector <davrecthreads at gmail.com>; Anastasia Stulova 
> <Anastasia.Stulova at arm.com>
> Cc: cfe-dev (cfe-dev at lists.llvm.org) <cfe-dev at lists.llvm.org>; 
> rjmccall at apple.com <rjmccall at apple.com>; nd <nd at arm.com>
> Subject: RE: [cfe-dev] [RFC] Re-use OpenCL address space attributes 
> for SYCL
>
>
> David's understanding is right. We would like to be able to call 
> existing C++
>
> functions from SYCL applications as much as possible.
>
>
>
> I don't know if analogy with the "const" qualifier is a perfect match 
> here, but
>
> casting a names address space pointer to "generic" pointer is valid 
> operation in
>
> all GPGPU programming models I'm aware of (including OpenCL). 
> "generic" means
>
> that memory can be allocation in any address space, so compiler can't 
> assume any
>
> specific address space and must generate code which is valid for any 
> address
>
> space. Usually it implies runtime overhead on checking the value of 
> "generic"
>
> pointer to handle it properly.
>
>
>
>> Alexey et al's alternative to prevent this breakage, if I understand
>
>> correctly, is to remove the type qualifier, and instead handle all 
>> address
>
>> space semantics in CodeGen (I believe this is what you refer to as 
>> keeping the
>
>> address space "flat").
>
>
>
> Not exactly. It works as what you described below - "the ideal". We 
> keep address
>
> space qualifier if user explicitly applied it, but the difference with 
> OpenCL
>
> mode is that "implicit" address space is the same as in regular C++ 
> (i.e.
>
> default) and we allow conversion to "default" C++ address spaces from 
> explicitly
>
> set named address spaces. C++ "default" is treated as "generic" w/o 
> changing C++
>
> default address space to OpenCL "generic" address space in Sema type 
> system.
>
> When SYCL users access memory though SYCL library API memory pointers 
> will be
>
> annotated with address space attributes to ensure good performance. 
> Users can
>
> obtain a "raw" pointer from SYCL objects to pass it to some generic 
> C++ function
>
> and this use case is enabled by the patch 
> https://reviews.llvm.org/D80932.
>
>
>
>> It seems to me that approach is not ideal, though, because
>
>>   a) it seems they would lose the type-checking benefits of 
>> implementing as a
>
>>  type qualifier (e.g. imagine if "const" qualifiers were removed and 
>> handled
>
>>  instead in CodeGen), and
>
>>  b) I think it really is important for the AST to maintain a clear
>
>>   representation of all target-independent semantics, including 
>> address
>
>>   spaces, so that users can easily reason about their code in AST 
>> matchers
>
>>   etc.
>
>
>
> Address space attributes are preserved in AST if they are applied 
> explicitly on
>
> source code, but they are not implicitly applied to all types.
>
> Type-checking is performed for OpenCL address space attributes (e.g. 
> casts
>
> between "named" address spaces are not allowed) and C++ type 
> qualifiers like
>
> "const" are respected.
>
>
>
>> So the ideal, it seems to me, for everyone’s sake, would be for 
>> OpenCL
>
>> qualifiers to behave more like "const" — there would be a default, 
>> a la
>
>> "non-const", that is applied to all types not explicitly qualified, 
>> so that
>
>> one could enable OpenCL mode and regular code would still work by 
>> default.
>
>
>
> I believe it's what we have in our implementation.
>
>
>
>> In reality though, I imagine this has all already been thought over
>
>> thoroughly, and it has been determined it really is unavoidable to 
>> break
>
>> standard C++ semantics in order to support address spaces; that there 
>> really
>
>> is no default that could be inferred for arbitrary types like those 
>> used in
>
>> template arguments.
>
>
>
>> But perhaps it is worthwhile to think it through one more time, to 
>> question
>
>> whether there may be some way to deduce type qualifiers properly in 
>> every
>
>> case, because the issue that Alexey et al raise is a good one I 
>> think.
>
>
>
> There is LLVM transformation pass which infers address space 
> information at LLVM
>
> IR level: https://llvm.org/doxygen/InferAddressSpaces_8cpp_source.html
>
> It helps to avoid performance overhead for regular C++ code called 
> from SYCL
>
> annotated parts.
>
>
>
> From: David Rector <davrecthreads at gmail.com>
> Sent: Thursday, July 30, 2020 2:33 AM
> To: Anastasia Stulova <Anastasia.Stulova at arm.com>
> Cc: Bader, Alexey <alexey.bader at intel.com>; cfe-dev 
> (cfe-dev at lists.llvm.org) <cfe-dev at lists.llvm.org>; rjmccall at apple.com; 
> nd <nd at arm.com>
> Subject: Re: [cfe-dev] [RFC] Re-use OpenCL address space attributes 
> for SYCL
>
>
>
> If I understand their proposal correctly, from the discussion Alexey 
> linked to in https://reviews.llvm.org/D80932#2073542, the primary 
> motivation for their implementation — its main distinction from 
> OpenCL’s approach -- is that they want to support address spaces in 
> such a way that existing C++ files without any address space markings 
> can still compile as is.
>
>
>
> That definitely seems like a worthy goal if it could be properly 
> accomplished.
>
>
>
> Take, for instance, the "const" qualifier.  Code which never uses it 
> whatsoever still works by default; only when we start adding "const" 
> into our code could we possibly start breaking other code.  That is 
> the ideal way to introduce a language feature: old code still works, 
> but now people can opt in to the new stuff.
>
>
>
> If instead const-ness had been implemented by allowing each type to be 
> either "const" or (let’s say) "mutable" *or neither*, and what is 
> more we implicitly added "mutable" when no such marking was provided 
> to some *but not all* types, then the users would not have the option 
> of ignoring it altogether, it would be a real headache.
>
>
>
> This seems to be how OpenCL is implemented, as Alexey et al identify 
> in the discussion linked above: certain VarDecl types get an implicit 
> __private qualifier, but e.g. template argument types, and certain 
> other types (they give another example beyond std::is_same which 
> presents problems) get no such implicit qualifier, resulting in 
> possible breakage in any code whose address spaces have not been 
> closely considered.
>
>
>
> Alexey et al's alternative to prevent this breakage, if I understand 
> correctly, is to remove the type qualifier, and instead handle all 
> address space semantics in CodeGen (I believe this is what you refer 
> to as keeping the address space "flat").
>
>
>
> It seems to me that approach is not ideal, though, because
>
>   a) it seems they would lose the type-checking benefits of 
> implementing as a type qualifier (e.g. imagine if "const" qualifiers 
> were removed and handled instead in CodeGen), and
>
>   b) I think it really is important for the AST to maintain a clear 
> representation of all target-independent semantics, including address 
> spaces, so that users can easily reason about their code in AST 
> matchers etc.
>
>
>
> So the ideal, it seems to me, for everyone’s sake, would be for 
> OpenCL qualifiers to behave more like "const" — there would be a 
> default, a la "non-const", that is applied to all types not explicitly 
> qualified, so that one could enable OpenCL mode and regular code would 
> still work by default.
>
>
>
> In reality though, I imagine this has all already been thought over 
> thoroughly, and it has been determined it really is unavoidable to 
> break standard C++ semantics in order to support address spaces; that 
> there really is no default that could be inferred for arbitrary types 
> like those used in template arguments.
>
>
>
> But perhaps it is worthwhile to think it through one more time, to 
> question whether there may be some way to deduce type qualifiers 
> properly in every case, because the issue that Alexey et al raise is a 
> good one I think.
>
>
>
> On Jul 29, 2020, at 5:42 PM, Anastasia Stulova 
> <Anastasia.Stulova at arm.com<mailto:Anastasia.Stulova at arm.com>> wrote:
>
>
>
>> I am not well-versed in this, but just thinking about these as 
>> arbitrary type qualifiers: could the issue be simply that the 
>> implicitly-generated address space qualifiers are *only* being added 
>> to the types of VarDecls, rather than to *every* type, including 
>> pointee types, template argument types, etc.?
>
>
>
> It is a little bit more complex than that. Most of the types used with 
> objects in OpenCL will get an address space deduced including pointer 
> types. This is because OpenCL is a language dialect for memory 
> segmented architectures and the memory segments pose some limitations 
> resulting in extra language rules. Clang strictly follows OpenCL 
> language spec and  I don't see any issue in the examples Alexey has 
> referred to. If the types differ by address space is_same is expected 
> to return false.
>
> What I struggle to understand how does this affects SYCL at all? The 
> deduction of address spaces is guarded by OpenCL language mode and it 
> is not set for SYCL as far as I am aware.
>
>
>
>> If it did, I believe those examples would all compile, and code would 
>> only break when the user specified began specifying non-default 
>> address spaces
>
>
>
> If I understand the design Alexey is proposing correctly the 
> user-specified address spaces are cast to the default address spaces 
> "hiddenly" and the AST always ends up to be in flat address space. 
> This is why I don't see the address space as a good fit. However, I am 
> not sure this is explained explicitly in the RFC, I might have 
> remembered this from some other discussions.
>
>
>
> ________________________________
>
> From: David Rector 
> <davrecthreads at gmail.com<mailto:davrecthreads at gmail.com>>
> Sent: 27 July 2020 22:32
> To: Bader, Alexey 
> <alexey.bader at intel.com<mailto:alexey.bader at intel.com>>
> Cc: Anastasia Stulova 
> <Anastasia.Stulova at arm.com<mailto:Anastasia.Stulova at arm.com>>; cfe-dev 
> (cfe-dev at lists.llvm.org<mailto:cfe-dev at lists.llvm.org>) 
> <cfe-dev at lists.llvm.org<mailto:cfe-dev at lists.llvm.org>>; 
> rjmccall at apple.com<mailto:rjmccall at apple.com> 
> <rjmccall at apple.com<mailto:rjmccall at apple.com>>; nd 
> <nd at arm.com<mailto:nd at arm.com>>
> Subject: Re: [cfe-dev] [RFC] Re-use OpenCL address space attributes 
> for SYCL
>
>
>
> On Jul 27, 2020, at 12:18 PM, Bader, Alexey via cfe-dev 
> <cfe-dev at lists.llvm.org<mailto:cfe-dev at lists.llvm.org>> wrote:
>
>>> I don't think (2) deal with language semantics. I assume we both 
>>> talking about
>
>>> the same case when variable declaration is not explicitly annotated 
>>> with address
>
>>> space attribute. According to language semantics such objects are 
>>> allocated in
>
>>> generic address space, but the problem is that most OpenCL 
>>> implementations have
>
>>> problems with consuming SPIR-V files with global variables in 
>>> generic address
>
>>> space. As an alternative to CodeGen changes we can consider handling 
>>> this issue
>
>>> in SPIR-V translator tool.
>
>>
>
>>
>
>> I am not really a CodeGen expert, maybe it will be ok. I think it's 
>> better if you discuss
>
>> it with John McCall or someone who is more experienced with CodeGen 
>> architecture.
>
>>
>
>> Why don't you just do regular address space deduction in Sema and 
>> then cast the
>
>> deduced address space to generic straight after? You already add 
>> similar casts for
>
>> pointers that are annotated with address spaces through the user 
>> code, right?
>
>> This approach will probably allow to reuse the logic from OpenCL and 
>> simplify CodeGen.
>
>
>
> I don't see how it can be done without breaking C++ semantics 
> demonstrated in
>
> https://reviews.llvm.org/D80932#2073542.
>
>
>
> I am not well-versed in this, but just thinking about these as 
> arbitrary type qualifiers: could the issue be simply that the 
> implicitly-generated address space qualifiers are *only* being added 
> to the types of VarDecls, rather than to *every* type, including 
> pointee types, template argument types, etc.?
>
>
>
> I.e., referring to the examples linked to above: perhaps the problem 
> is *not* that that OpenCL changes `int var` to `__private int var`, 
> but rather that it does not *also* change `int* ptr1 = &var` to 
> `__private int* __private ptr1 = &var` (or whatever the proper default 
> qualifiers are) and `std::is_same<T, int>` to `std::is_same<T, 
> __private int>` when in OpenCL (or SYCL) mode.
>
>
>
> If it did, I believe those examples would all compile, and code would 
> only break when the user specified began specifying non-default 
> address spaces, i.e. when they actually used the feature in their 
> code.  In this way, the non-standard semantics could be represented in 
> the AST without affecting the standard semantics.
>
>
>
> In any case that is the form of the ideal solution: sure, don’t 
> break the standard C++ semantics, but also, try to keep a clear 
> representation of any supported-but-non-standard semantics in the AST, 
> I think.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200806/4102a987/attachment-0001.html>