erichkeane 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'm glad to hear that! I'm hopeful it isn't TOO much of a delay and not too much work. Sema.h changes are at least pretty innocuous/small. > > 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 agree with the goal as well! > > 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. > Urgh, and double-urgh. Yeah, not big enough/important enough, we only ever instantiate a handful of Sema objects anyway at most. > > 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. "Commonly used" stuff together perhaps has some value for cache-locality reasons (as well as any other "closely related" stuff with itself), but I don't have a good way to measure that so 'best effort' here is probably good enough/the changes you're intending a good enough attempt. 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
