erichkeane added a comment.

Response on CGFucntionInfo.


================
Comment at: include/clang/CodeGen/CGFunctionInfo.h:479
@@ +478,3 @@
+  /// Whether this function saved caller registers.
+  unsigned NoCallerSavedRegs : 1;
+
----------------
aaron.ballman wrote:
> This is unfortunate as it will bump the bit-field length to 33 bits, which 
> seems rather wasteful. Are there any bits we can steal to bring this back 
> down to a 32-bit bit-field?
I implemented this additional patch, but don't really know a TON about this 
area, so I have a few ideas but would like to get direction on it if possible.  
I had the following ideas of where to get a few more bits:

CallingConvention/EffectiveCallingConvention/ASTCallingConvention:  This 
structure stores a pre-converted calling convention to the llvm::CallingConv 
enum (llvm/include/llvm/IR/CallingConv.h).  I'll note that the legal values for 
this go up to 1023 (so 8 bits isn't enough anyway!), though only up to 91 are 
currently used.  

HOWEVER, the clang CallingConv (include/clang/Basic/Specifiers.h :233) only has 
16 items in it.  If we were instead to store THAT and convert upon access (a 
simple switch statement, already used constructing this value, see 
ClangCallConvToLLVMCallConv), we could get away with 6 or 7 bits each, saving 
this 3-6 bits total, yet have way more than enough room for expansion.

HasRegParm: This field might be possible to eliminate.  According to the GCC 
RegParm docs (I don't see a clang one?) here 
(https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html), ", the 
regparm attribute causes the compiler to pass arguments number one to number if 
they are of integral type in registers EAX, EDX, and ECX instead of on the 
stack".

It seems that 0 for "RegParm" should be illegal, so I wonder if we can treat 
"RegParm==0" as "HasRegParm==false" and eliminate storing that.

In my opinion, the 1st one gives us way more room for the future and corrects a 
possible future bug.  The 2nd is likely a lower touch, though it could possibly 
change behavior (see discussion here 
https://www.coreboot.org/pipermail/coreboot/2008-October/040406.html) as 
regparm(0) is seemingly accepted by both compilers.  I DO note that this 
comment notes that 'regparm 0' is the default behavior, so I'm not sure what 
change that would do.

Either way, I suspect this change should be a separate commit (though I would 
figure making it a pre-req for this patch would be the right way to go).  If 
you give some guidance as to which you think would be better, I can put a patch 
together.

-Erich



https://reviews.llvm.org/D22045



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

Reply via email to