[cfe-dev] [llvm-dev] Proposal for string keys for address_space

Fri Jan 11 11:01:42 PST 2019

On Fri, Jan 11, 2019 at 10:36 AM Leonard Chan via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> @Anastasia
>
> > Just to understand better on the language side do you suggest to keep
> integer value for an attribute too? I think for the legacy or speed reason
> some users might still prefer integers.
>
> Yup. I should've clarified on this, I intend to also keep the existing
> integer parameters. It's just that no string address space corresponds
> to any integer address space.
>
> > In Clang AST address spaces are stored in Qualifiers as integer values.
> So you would need some conversion and a map from string to integers to be
> stored. ASTContext could be a good place to store the map since it's
> accessible during all the different phases - parsing, sema, codegen.
>
> I see that this map could be stored in ASTContext. Currently, I think
> it's more a matter of accessing this map in the TypePrinter. I think
> the printer is accessed mostly through various getAsString() methods
> of a QualType but these do not access ASTContext or Sema in any way. I
> think this would require refactoring a bunch of refactoring for how
> types can be printed to expose ASTContext to the TypePrinter.
> Alternatively, since the address space is only stored in the
> Qualifiers class as part of a mask, instead Qualifiers could have some
> reference to some abstract representation of an address space which
> could represent either an integer or string. But these are just some
> of my ideas and would like to know if there are better ways of
> approaching this.
>
> > Btw, are you suggesting this only for Clang? Other than emitting the
> address space map (as metadata perhaps?) in IR, are there any other changes
> you plan?
>
> Yup, this is only meant to be a clang feature. Should've clarified on
> this also. We have no intentions on changing llvm or dumping this map
> as metadata.
>

What integer are you emitting in LLVM for a given string?
I assume we need a deterministic and immutable mapping if you don't
propagate the string to LLVM.

-- 
Mehdi

> On Fri, Jan 11, 2019 at 9:04 AM Justin Lebar <jlebar at google.com> wrote:
> >
> > I don't (at the moment) have an opinion on the implementation, but to
> the broader principle of improving the representation of these address
> spaces, this sounds like it would scratch an itch of mine.  In XLA nvptx
> backend, passing around these integer address spaces has lead to a few
> near-bugs, and when reading nvptx IR, it would be super-nice if I could
> somehow be reminded of what address space 5 is.
> >
> > -Justin
> >
> > On Fri, Jan 11, 2019 at 6:21 AM Anastasia Stulova via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
> >>
> >> Just to understand better on the language side do you suggest to keep
> integer value for an attribute too? I think for the legacy or speed reason
> some users might still prefer integers.
> >>
> >>
> >> In Clang AST address spaces are stored in Qualifiers as integer values.
> So you would need some conversion and a map from string to integers to be
> stored. ASTContext could be a good place to store the map since it's
> accessible during all the different phases - parsing, sema, codegen.
> >>
> >>
> >> Btw, are you suggesting this only for Clang? Other than emitting the
> address space map (as metadata perhaps?) in IR, are there any other changes
> you plan?
> >>
> >>
> >> Cheers,
> >>
> >> Anastasia
> >>
> >>
> >> ________________________________
> >> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Jacob
> Lifshay via llvm-dev <llvm-dev at lists.llvm.org>
> >> Sent: 10 January 2019 23:13
> >> To: Leonard Chan
> >> Cc: llvm-dev; Clang Dev
> >> Subject: Re: [llvm-dev] Proposal for string keys for address_space
> >>
> >> I don't know about clang, but it should work for llvm.
> >>
> >> On Thu, Jan 10, 2019, 14:54 Leonard Chan <leonardchan at google.com wrote:
> >>
> >> @Jacob Lifshay
> >>
> >> In the case of the TypePrinter though, is there a way to access
> >> ASTContext? Part of my dilemma is that for diagnostics that simply
> >> print the type or the qualifiers, there doesn't seem to be an
> >> established way to access Context so I wouldn't be able to access this
> >> mapping. I'm thinking either this will require some refactoring of how
> >> types are printed to expose ASTContext or perhaps Qualifiers could be
> >> changed somehow such that the 23 bits allocated for the address space
> >> could instead point to some representation of an address_space that in
> >> turn could represent either a string or integer.
> >>
> >> - Leonard
> >>
> >> On Thu, Jan 10, 2019 at 2:21 PM Leonard Chan <leonardchan at google.com>
> wrote:
> >> >
> >> > +cfe-dev at lists.llvm.org
> >> >
> >> > On Thu, Jan 10, 2019 at 2:16 PM Jacob Lifshay <
> programmerjake at gmail.com> wrote:
> >> > >
> >> > > Stash a lookup table from integers to strings in Context and
> dynamically allocate integers for new strings. You can then keep integers
> in most of the code, writing/displaying strings for the integers with an
> entry in the table when writing to files or displaying.
> >> > >
> >> > > Jacob Lifshay
> >> > >
> >> > > On Thu, Jan 10, 2019, 13:54 Leonard Chan via llvm-dev <
> llvm-dev at lists.llvm.org wrote:
> >> > >>
> >> > >> Hello,
> >> > >>
> >> > >> We would like to propose a way for improving the diagnostics for
> >> > >> address_space by being able to pass strings as an argument to it
> >> > >> instead of just an integer. This was initially proposed before
> >> > >> (http://lists.llvm.org/pipermail/cfe-dev/2018-August/058702.html)
> but
> >> > >> did not focus on it at the time.
> >> > >>
> >> > >> Reasoning:
> >> > >> Clang's __attribute__((address_space(...))) feature uses an
> arbitrary
> >> > >> integer as the discriminator of address spaces. This is
> effectively a
> >> > >> global name space of integers in an API. The meaningless integers
> are
> >> > >> used in diagnostics, which is not very informative. Moreover, the
> >> > >> maintenance of the integer assignments is error-prone. It would be
> >> > >> better if this could be a string rather than an integer. It's easy
> >> > >> enough to pick string prefixes that distinguish the use in one
> part of
> >> > >> an API from others in separately-maintained APIs, just as with
> symbol
> >> > >> name prefixes in C APIs.
> >> > >>
> >> > >> Goals:
> >> > >> - Allow for address_space to accept strings so various APIs do not
> >> > >> need to share the same global name space of integers
> >> > >> (address_space("API1") != address_space("API2"))
> >> > >> - Print the string argument instead of an integer if address_space
> is
> >> > >> provided with a string for more understandable error messages.
> >> > >>
> >> > >> (Possible) Implementation:
> >> > >> Off the top of my head, it seems that a large chunk of this would
> be
> >> > >> changing how address spaces are managed. Currently, it seems that
> all
> >> > >> address_spaces are limited to just integers with a few spaces
> reserved
> >> > >> for OpenCL and CUDA under the LangAS enum. I'm thinking that LangAS
> >> > >> itself could be a base class with 2 different subclasses that take
> >> > >> either integers or strings and when storing them in a qualifier,
> part
> >> > >> of the address space mask could be dedicated to indicating if this
> >> > >> LangAS is a string or integer. The only thing I can't think of is
> how
> >> > >> printing the AS would work in a diagnostic since, based off the
> >> > >> existing TypePrinter/Qualifiers::print() method, I do not think I
> >> > >> would be able to store a reference back to some mapping of addr
> spaces
> >> > >> to strings since it seems that the Qualifiers class is meant to be
> 32
> >> > >> bits long and passed by value.
> >> > >>
> >> > >> I was going to come up with a proof of concept in the meantime, but
> >> > >> wanted to ask sooner to see if anyone had ideas on this or ideas on
> >> > >> how this could be implemented. Any feedback is welcome and I don't
> >> > >> mind answering questions.
> >> > >>
> >> > >> Thanks,
> >> > >> Leonard
> >> > >> _______________________________________________
> >> > >> LLVM Developers mailing list
> >> > >> llvm-dev at lists.llvm.org
> >> > >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >>
> >> _______________________________________________
> >> 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/20190111/8ca1b06e/attachment.html>