erichkeane added inline comments.

================
Comment at: include/clang/CodeGen/CGFunctionInfo.h:519
+  /// Whether this function has nocf_check attribute.
+  unsigned NoCfCheck : 1;
+
----------------
aaron.ballman wrote:
> oren_ben_simhon wrote:
> > aaron.ballman wrote:
> > > This is unfortunate -- it bumps the bit-field over 32 bits. Can the bit 
> > > be stolen from elsewhere?
> > The field is orthogonal to the other fields moreover i think that double 
> > meaning of the same field will lead to future bugs. The class is not a 
> > compact packed structure, so i don't feel it worth the confusion.
> The class packs its fields for space savings because it's used fairly 
> frequently; if we can keep the size from having to use another allocation 
> unit, that's better. I wasn't suggesting we make a bit have double meaning, I 
> was wondering if we could reduce the size of one of the other fields. For 
> instance, I'd be surprised if we need all 8 bits for calling conventions, so 
> we might be able to reduce that to a 7-bit field.
Note that LLVM::CallingConv 
(http://llvm.org/doxygen/namespacellvm_1_1CallingConv.html) only uses up to 96, 
yet has a 'MaxID' of 1023 for some reason.  It seems that backing this down to 
255 would be more than reasonable and would prevent issues here, but wouldn't 
save any bits here.  We could possibly make it '127' in LLVM, but I fear that 
would limit the values pretty harshly.

HOWEVER, clang::CallingConv 
(https://clang.llvm.org/doxygen/Specifiers_8h_source.html#l00233) only has 17 
items.  Because of this, 7 bits is 1 more than necessary.  We could save a bit 
from ASTCallingConvention, and still permit nearly doubling calling 
conventions. 


Repository:
  rL LLVM

https://reviews.llvm.org/D41880



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

Reply via email to