rjmccall 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()
----------------
rogfer01 wrote:
> 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.
`CGFunctionInfo` should be totally immutable and just ultimately constructed
with an array of `ABIArgInfo`, which seems like a reasonable place to inject
some representation tricks.
================
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:
> 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 :(
You could gate it on the compiler if you want, maybe extracting a
`HAS_IS_TRIVIALLY_COPYABLE` test macro in `llvm/Support/type_traits.h` out of
the implementation of `llvm::isPodLike`.
https://reviews.llvm.org/D48589
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
