[PATCH] D66197: AMDGPU: Add intrinsics for address space identification

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D66197#1629896 <https://reviews.llvm.org/D66197#1629896>, @jdoerfert wrote:

> In D66197#1629864 <https://reviews.llvm.org/D66197#1629864>, @arsenm wrote:
>
> > In D66197#1629861 <https://reviews.llvm.org/D66197#1629861>, @jdoerfert wrote:
> >
> > > In D66197#1629809 <https://reviews.llvm.org/D66197#1629809>, @b-sumner wrote:
> > >
> > > > In D66197#1629784 <https://reviews.llvm.org/D66197#1629784>, @jdoerfert wrote:
> > > >
> > > > > Do we really need these to be "amdgpu" specific?
> > > >
> > > >
> > > > Are you envisioning these would be used for OpenCL implementations?  OpenCL doesn't exactly have these.  It instead has to_<addrspacename> functions which return NULL if the generic pointer isn't actually pointing at a object in <addrrspacename>.
> > >
> > >
> > > We (will) have various languages/targets that have corresponding address spaces and we already reserve some numbers for specific address spaces (afaik), why not make this a generic functionality.
> >
> >
> > The IR doesn't reserve any numbers for specific usage (except 0 has some special properties, which do not include being a flat/generic pointer as defined in OpenCL). It might make more sense to add a clang builtin for this, which could then be implemented with the target specific intrinsic. I don't want to add a generic target intrinsic while guessing at how this might work on other targets. Something truly generic, like llvm.is.address.space(ptr, address_space_id) I don't think really works generally enough to add. There isn't necessarily a 1:1 mapping between the language address space and IR address space. There could possibly be multiple IR address spaces to handle, and not all targets might be able to do this test at all
>
>
> I see, still, we have `llvm.nvvm.isspacep.const` and friends already. Now we get `llvm.amdgcn.is.private`, which seems to be the same thing for amdgpu. As long as people only use this in the backend, great, but if we want middle-end passes that deal with address spaces and optimize accordingly, e.g., introduce data movement, we should have generic intrinsics or helper functions. I would prefer the former and I was curious if there is a problem with that. If the folding (I described earlier) is triple/target specific, sure, but if we do not have multiple `llvm.XYZ.is.private` we would simplify things.


I'm not sure what you mean by data movement. InferAddressSpaces handles this in this patch when the source is inferred. I don't think there are any other passes that would ever need to care about this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66197/new/

https://reviews.llvm.org/D66197