aaron.ballman added inline comments. ================ Comment at: include/clang/CodeGen/CGFunctionInfo.h:479 @@ +478,3 @@ + /// Whether this function saved caller registers. + unsigned NoCallerSavedRegs : 1; + ---------------- erichkeane wrote: > 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. I'm on the fence about 6 vs 7 and see no harm in reducing it to either value, but have a *very* slight preference for 7 so that the bit-field grouping continues to add up to 32-bits. However, it's your call.
https://reviews.llvm.org/D22045 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
