Chandler Carruth wrote:
> I understand that we need some way of handling inttoptr stuff, but I can
> see good ways of doing this even with an "opt in" strategy using
> We could auto-upgrade code to addrspace cast to address space 0 and then
> do ptrtoint (and vice versa).
> But I'm all in favor of making this change, just arguing that the
> semantics should be the other way around.
In the current patch it may be difficult to come up with a safe
fallback that works for everyone. For instance, addrspacecast to
addrspace(0) will not work for address spaces that have pointers
larger than addrspace(0) pointers.
*However*, one possible migration path will become possible once we
add the "@llvm.ptrtoint" intrinsic, if we also add an "@llvm.inttoptr"
intrinsic. Once we have those two, we should be able to auto-upgrade
`inttoptr` and `ptrtoint` instructions to calls to these intrinsics;
assuming we can figure out some way to deal with ConstantExprs. I
think expanding ConstantExprs that use (now illegal) `ptrtoint` or
`inttoptr` instructions into instructions in each use site will work,
but I have to think about it a bit more.
> > So I don't think you need a tagged address space to implement
> > here, and I'd like to avoid tagging the address space until the last
> > possible second to make sure that this is implemented as
> generically as
> > possible. I'm actually hopeful that the tagging isn't necessary
> at all.
> Won't that be stalled on every user of non-zero address spaces
> agreeing on these semantics (e.g. on that they don't need `inttoptr`
> for instance)? No one has so far stepped forth to discuss the
> semantics they need -- though this could have to do with the subject
> line though.
> I can call the tag something more generic than "GC" if you wish (in
> fact, that was the reason for keeping the first patch trivial, to hash
> these things out ), but it seems friendlier to start with
> explicitly enumerating the address space(s?) that need the
> no-integer-conversion properties, and flip the default once the
> changes have circulated more widely and people have had a chance to
> Given how few users of non-zero address space pointers there are I
> actually am not sure about this. Maybe you do need a separate thread to
> make it clear that we're considering changing the defaults for address
> spaces, but I personally much prefer the constructively correct approach
> where once you are outside of address space zero, there *is* no integer
> mapping available unless people need one and opt into having that behavior.
> There are relatively few such users though, so i feel like you could
> likely start a fresh thread and collect them pretty quickly if this in
> practice isn't the right tradeoff.
> Either way the defaults go, I'd also prefer that the description be
> about the semantics not about the use case. So I'd suggest a flag
> indicating an address space has an integer mapping (or that it doesn't
> have such a mapping) rather than one that talks about GC.
I've s/GC reference/non-integral pointer/ in the review.
Flipping the default is may be the right thing to do eventually, but I
think doing that now as a *first step* makes the current change
unnecessarily invasive. I think the right time to talk about flipping
the default is once all the key pieces are in place, and we have
reason to believe that the new code is not completely, utterly broken
How about this plan:
- I'll start (yet!) another RFC on on llvm-dev specifically asking
about the use cases people have for non-0 address spaces. I'll use
a non-GC specific title and explicitly CC some people I know use
non-0 address spaces.
- I'll try to push for getting all of the "no integer <-> pointer
conversions" changes checked in one by one, all still predicated on
an "opt-in" non-integral address space.
- Once we are reasonably confident that we have stands up, depending
on what we discover from the RFC mentioned in the first step we
will flip the default (or not). As discussed in the beginning of
the email, a forward migration path should not be too difficult as
long as we have both `@llvm.inttoptr` and `@llvm.ptrtoint`.