This is an archive of the discontinued LLVM Phabricator instance.

[docs][OpaquePtr] Shuffle around the transition plan section
ClosedPublic

Authored by aeubanks on Jun 11 2021, 1:35 PM.

Details

Summary

Emphasize that this is basically an attempt to remove
`PointerType::getElementType and Type::getPointerElementType()`.

Add a couple more subtasks.

Diff Detail

Event Timeline

aeubanks requested review of this revision.Jun 11 2021, 1:35 PM
aeubanks created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2021, 1:35 PM
dblaikie accepted this revision.Jun 11 2021, 1:59 PM

Sounds good to me - couple of optional comments/questions that could be clarified in the text.

llvm/docs/OpaquePointers.rst
118

May be confusing to say "This may already be done" when the line before sort of seems like it says "this is already done".

Maybe rephrase this to "More cases may be found as work continues"?

140–143

Oh, yeah. I remember that one.

Are there cases you know of where we will need overloading based on pointee type? Or might it be a case of mangling opaque pointer types as opaque being sufficient?

This revision is now accepted and ready to land.Jun 11 2021, 1:59 PM
aeubanks marked an inline comment as done.Jun 11 2021, 3:09 PM
aeubanks added inline comments.
llvm/docs/OpaquePointers.rst
140–143

So far I've found D61524 that needs to be done.
But I'm confused about your question. We won't have pointee types, so we won't be able to mangle based on pointee type, we can only mangle opaque pointers as opaque pointers (with their address space).

dblaikie added inline comments.Jun 11 2021, 3:24 PM
llvm/docs/OpaquePointers.rst
140–143

What I mean is are there intrinsics that have different behavior based on the pointee type (ie: a function that takes only a pointer parameter, but actually has different behavior (I could imagine something like a typed memcpy where the type information (size of elements to copy) is communicated through the pointee type)) - or, to the best of our current knowledge, the intrinsic overloading for pointee types is only because it's necessary for those functions to have different names to account for the different parameters (and not require bitcasts for every parameter) but have the same implementation.

aeubanks added inline comments.Jun 14 2021, 10:59 AM
llvm/docs/OpaquePointers.rst
140–143

Ah I haven't looked through all of the intrinsics, I've only stumbled upon D61524 so far, which isn't exactly specializing implementations per pointee type, rather just looking at the pointee type.

dblaikie added inline comments.Jun 14 2021, 9:12 PM
llvm/docs/OpaquePointers.rst
140–143

Oh, so in that case it is specialized in the sense that it does do something different, by the sounds of it, depending on the type? (where "different" is "touches more or fewer bytes based on the size of the pointee type" - same algorithm, but not exactly the same instructions - that's "different" enough for what I'm thinking about/getting at)

So in that case it won't be as simple as updating the mangling code to do the right thing for opaque pointers (I see there's another review out to make that change, which seems good) - but it'll need to deliberately mangle in a type that isn't a parameter type (I don't know if there's precedent for that - hopefully it's not too complicated) into the function name, and recover that to do the gep-like thing?

aeubanks added inline comments.Jun 15 2021, 12:02 PM
llvm/docs/OpaquePointers.rst
140–143

I think mangling is purely based on argument types, you currently can't encode a random type into an intrinsic, so there isn't any precedent.
A random thought I had was to add a dummy parameter with the underlying GEP type (and just pass undef into it) so we can retrieve the type later, but perhaps there's a better way.