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: > erichkeane wrote: > > 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 > > > I think that `unsigned ASTCallingConvention : 8;` can be safely reduced. This > tracks a `clang::CallingConv` value, the maximum of which is 15. So I think > the way to do this is to reduce ASTCallingConvention from 8 to 7 bits and > then pack yours in as well. Ah! I missed that this was the case. That said, it could likely be reduced to 6 if we really wished (currently 16 items, 6 gives us room for 32). Perhaps something to keep in our pocket for the next time someone needs a bit or two here. I'll update the diff for Amjad if possible. https://reviews.llvm.org/D22045 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
