Endilll wrote:

@erichkeane 

> That said, waiting until after 18 is perhaps a good diea.

Resolving merge conflicts that will arise in the meantime is not going to be 
trivial, but should be doable in a reasonable time. So I'm willing to wait.

> I MIGHT suggest private followed by public? It is a not-uncommon pattern I've 
> seen to have a private 'helper' class(or data member) defined inline, that is 
> then used by inline-defined public functions.

I don't mind either way, as long as we don't inserting a couple of private 
members in the middle of public section.

> I wouldn't mind some sort of 'static_assert' to ensure that this doesn't 
> accidentally increase the size of Sema

Checking on Linux, `sizeof(Sema)` is 18824 bytes at moment. This patch 
increases that by 24 bytes, to `18848`. I don't deem it significant enough to 
care for an object this big, but I can claw it back if this is considered 
important.

> or cause some sort of pessimization for layout. I realize we're not 
> particularly concerned about the size, but I could imagine goofy things going 
> on.

If there was an intent to put some data members first to improve cache hits, I 
think it has been lost by this point. Our first non-static data members are 
`OpenCLOptions OpenCLFeatures` and `FPOptions CurFPFeatures` at the moment. 
Since this patch puts generic Sema stuff (`Sema.cpp`) first, opportunity to put 
widely used data members first is still there. As I mentioned in the 
description, follow-up patches that improve ordering in each particular section 
are expected.

https://github.com/llvm/llvm-project/pull/82217
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to