majnemer added a subscriber: majnemer. ================ 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: > > > 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. I'd go with 6 and have another bitfield, `unsigned UnusedBits : 1;`
We use this paradigm elsewhere. https://reviews.llvm.org/D22045 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
