[PATCH] D38473: Include getting generated struct offsets in CodegenABITypes

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 4 19:32:59 PDT 2017


rjmccall added a comment.

In https://reviews.llvm.org/D38473#888597, @rsmith wrote:

> In https://reviews.llvm.org/D38473#888159, @mppf wrote:
>
> > > You should also indicate *which* record layout (complete object type or base subobject type) the field number is for. I don't think there's any guarantee that the same indexes work in both.
> >
> > I added a comment about this - indicating that inherited fields are not supported by this function. My use case is just for C structs at the moment and I'm not actually sure what we'd need to do for the interface to support building GEPs for inherited fields. I'm sure it would complicate the API. I think we should leave that complexity to another function or a future function (when somebody actually needs it).
>
>
> I don't think your added comment is related to my request. There are two (potentially) different LLVM types for a C++ class type in general, one for the case where the object is a base class subobject (excluding vbases, and where the tail padding could be reused) and one for where it is a complete object (including vbases, and where the tail padding is known to be owned by the object). I don't know whether we guarantee that field indexes for one layout can be used for the other layout; in particular, I'm concerned that one could be a packed struct when the other isn't, and thus the field indexes could differ. (I think we generally use the complete object type within IR generation, even though that may seem like an odd choice.)
>
> But perhaps we do guarantee the common prefixes are the same (@rjmccall: do we?), or we only ever expose one of the two layouts to code outside Clang, and in either case this wouldn't matter.


I think we should probably just document that they're only valid in the complete-object type, which also, yes, happens to be the only type we expose outside of Clang because it's the type returned by convertTypeForMemory.

> 
> 
>>> (Eg, if we move to an IR representation using something non-GEPable to represent a source-level class type, we would remove this function as it would be meaningless.)
>> 
>> In that event, I'd just want to have a way to build the appropriate field access. (e.g. a function to build an appropriate GEP; or a function that adds appropriate instructions to an IRBuilder and returns a Value* that points to the field, say). I'm open to trying to go directly to such an interface but I'd need significant help in designing/implementing it. (The interface I've proposed here meets my needs and I don't think I know enough about clang to design/implement a better one without help).
> 
> Given the generality of kinds of lvalues and rvalues that we support, this would mean exposing a significant chunk of the CodeGen interface, I think -- probably much more than we would be comfortable exposing.

I thought about making this sort of comment and decided that if we added more non-GEPable kinds of fields, we could always just put them after bit-fields in the exceptions list.


https://reviews.llvm.org/D38473





More information about the cfe-commits mailing list