rogfer01 added inline comments.
================ Comment at: include/clang/CodeGen/CGFunctionInfo.h:90 union { - unsigned DirectOffset; // isDirect() || isExtend() - unsigned IndirectAlign; // isIndirect() - unsigned AllocaFieldIndex; // isInAlloca() + llvm::StructType *ExtendSeq; // isCoerceAndExpand() + unsigned DirectOffset; // isDirect() || isExtend() ---------------- rjmccall wrote: > Hmm. I understand the need to use something uniqued here, but I think it > would probably be more natural to at least use a `llvm::ConstantDataArray` > (which could be null if there are no interesting bits) instead of encoding > the information into the types of a struct type. That would also make it > easy to generalize the elements to an arbitrary flags type. > > Also, on 64-bit targets this will increase the size of an `ABIArgInfo` to > four pointers from three. That's fine to the extent that we work with > independent `ABIArgInfo`s, but I'm getting a little annoyed at the storage > overhead of the array of `ABIArgInfo`s in `CGFunctionInfo` given that, in the > overwhelmingly common case, an `ABIArgInfo` is no more than a kind and maybe > a few of these flags. Maybe there's some reasonable way to optimize the > storage of an `ABIArgInfo` in a `CGFunctionInfo` so that we only need the > extra storage in less-common cases? Like extracting out a base class that's > the Kind+Flags and making the main array be an array of those + an optional > index into a second trailing array of full `ABIArgInfo`s. > > I might be overthinking this, though. Oh, of course, using a value rather than a type is more sensible. I will change this. The idea of having a lighter `ABIArgInfo` and an ancillary extra array for it seems sensible. I guess it may depend on how early/late we know the precise kind of `ABIArgInfo` we need to represent. I haven't looked at this yet, I'll investigate. ================ Comment at: include/clang/CodeGen/CGFunctionInfo.h:475 +static_assert(std::is_trivially_copyable<ABIArgInfo>::value, + "ABIArgInfo must be trivially copyable as it is embedded as trailing " ---------------- rogfer01 wrote: > I think this is the right trait here. I spent too much time debugging this :) But today I realised that GCC 4.9 does not implement this so it will have to go away :( https://reviews.llvm.org/D48589 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits