[cfe-dev] address_space pragma

Leonard Chan via cfe-dev cfe-dev at lists.llvm.org
Thu Aug 9 11:46:50 PDT 2018


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
> >>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180809/aa14928d/attachment.html>


More information about the cfe-dev mailing list