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()
----------------
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.


https://reviews.llvm.org/D48589



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to