[cfe-dev] address_space pragma

Thu Aug 9 13:02:19 PDT 2018

It'd probably be wise to wait for https://reviews.llvm.org/D50526 to land
first, since that's going to cause you significant merge pain.

On Thu, 9 Aug 2018 at 11:47, Leonard Chan via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> O right. My bad, I'll work on this first before moving onto parsing the
> macro tokens.
>
> On Thu, Aug 9, 2018, 11:42 John McCall <rjmccall at apple.com> wrote:
>
>> > On Aug 9, 2018, at 1:27 PM, Leonard Chan <leonardchan at google.com>
>> wrote:
>> >
>> > Right now, the goal is to just change the diagnostic that would
>> > normally be printed for mixing address spaces to instead print the
>> > macro in place of the whole attribute that contains this address
>> > space. That is:
>> >
>> > ```
>> > # define __user __attribute__((noderef, address_space(1)))
>> >
>> > char __user *x = ...
>> > char *y = x;  // err: initializing 'char *' with an expression of type
>> > '__attribute__((address_space(1))) char *' changes address space of
>> > pointer
>> > ```
>> >
>> > The error would instead print `initializing 'char *' with an
>> > expression of type '__user char *' changes address space of pointer`.
>> >
>> > It seems that the address space is attached to the type via a
>> > qualifier instead of the type, that is, I don't think something like
>> > `__attribute__((address_space(10))) int *` generates an
>> > AttributedType. This makes me think that I need to attach something to
>> > the Qualifier, but I don't think I want to increase how much it holds
>> > since it gets copied everywhere and only holds a mask of 32 bits.
>> >
>> > The only alternative options I can think of is changing the printing
>> > policy to accept information relating to the macro, but am unsure if
>> > this is the right solution.
>>
>> Well, like I said before:
>>
>> > I'm not sure we actually make an AttributedType for address_space, but
>> doing
>> > so is reasonable.
>>
>> AttributedType wasn't part of the original AST for sugared types; IIRC we
>> added it
>> in 2011 or so, and we didn't update most of the existing places where
>> attributes
>> modify types.  It should be pretty straightforward to do that for
>> address_space.
>>
>> John.
>>
>> > On Mon, Aug 6, 2018 at 2:26 AM Bevin Hansson <
>> bevin.hansson at ericsson.com> wrote:
>> >>
>> >> Hi,
>> >>
>> >> I'm a bit late to the party, but all this address space stuff sounds
>> >> interesting to me as we are heavy users of address space functionality
>> >> downstream.
>> >>
>> >> If I understand correctly, you want to include the spelling of the AS
>> in
>> >> the type. Would this be restricted purely to spellings as macros, or
>> >> would it also work for keywords? There are a bunch of hard coded
>> strings
>> >> in the type printer to print out the specific names of ASes for OpenCL,
>> >> and we have also added our ASes to that hack. If it were possible to
>> >> encode this straight in the type itself based on keyword/identifier
>> >> instead of special-casing in the printer, that would probably be
>> cleaner.
>> >>
>> >> Feel free to add me as reviewer if you make a patch.
>> >>
>> >> / Bevin
>> >>
>> >>
>> >> On 2018-08-03 01:49, Leonard Chan via cfe-dev wrote:
>> >>> As I have it now, I store a map of source locations to macro
>> >>> definition names as they are being parsed and that's where I attempt
>> >>> to find the attribute. Since the printing of the address_space is
>> >>> controlled by the qualifier, I added an extra field to the qualifier
>> >>> instead for storing the macro name. I can get this name during the
>> >>> address space attribute construction where the source location of
>> >>> address_space is available to me.
>> >>>
>> >>> Would you mind if I added you as a reviewer to this patch after I
>> clean it up?
>> >>> On Thu, Aug 2, 2018 at 3:38 PM John McCall <rjmccall at apple.com>
>> wrote:
>> >>>>> On Aug 2, 2018, at 5:41 PM, Leonard Chan <leonardchan at google.com>
>> wrote:
>> >>>>>
>> >>>>> On Thu, Aug 2, 2018 at 2:33 PM John McCall <rjmccall at apple.com>
>> wrote:
>> >>>>>>> On Aug 2, 2018, at 5:28 PM, Leonard Chan <leonardchan at google.com>
>> wrote:
>> >>>>>>>
>> >>>>>>> Ok. So we will no longer use the pragma. We will instead opt for
>> your
>> >>>>>>> suggestion of printing the macro. So the new goals are now:
>> >>>>>>>
>> >>>>>>> - Having address_space() accept strings in addition to integers to
>> >>>>>>> represent unique address spaces
>> >>>>>> Is this still important if you get the diagnostic improvement?  If
>> you were declaring address
>> >>>>>> spaces somehow, this seems fine, but I don't really want to make
>> address_space("whatever")
>> >>>>>> implicitly declare an address space with specific semantics when
>> it's not hard to imagine
>> >>>>>> wanting that for other purposes in the future.
>> >>>>> Not entirely. This was something that's still remnant of when we
>> were
>> >>>>> using pragma. It's something that would be nice for us now, but
>> could
>> >>>>> also be discussed later down the line.
>> >>>>>
>> >>>>>>> - Changing the type printer to print the macro instead of the
>> whole
>> >>>>>>> attribute if the attribute was passed a string instead of an int.
>> >>>>>> I don't know that the latter needs to be restricted; it seems like
>> a nice usability improvement
>> >>>>>> for all address-space users.
>> >>>>> Ok, so in general if address_space is used in a macro, then print
>> the macro.
>> >>>> Yeah.  You'll need to record this spelling information in the
>> AttributedType, since
>> >>>> the Type won't otherwise have a source location attached.
>> >>>>
>> >>>> I'm not sure we actually make an AttributedType for address_space,
>> but doing
>> >>>> so is reasonable.
>> >>>>
>> >>>> We should do a quick parse of the macro's token sequence to verify
>> that it's just
>> >>>> an attribute.  Unfortunately, I'm not really sure how to do that.
>> >>>>
>> >>>> John.
>> >>> _______________________________________________
>> >>> cfe-dev mailing list
>> >>> cfe-dev at lists.llvm.org
>> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>> >>
>>
>> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180809/c391e3b0/attachment.html>