[PATCH] D125635: Move NumStmts from CompoundStmtBitfields to fields of CompoundStmt

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun May 15 15:04:38 PDT 2022


rjmccall added a comment.

On the one hand, I'm not sure 8M statements in a block — or 1M, for that matter — is an unreasonable implementation limit.  On the other hand, this patch looks like it only changes overall memory use on 32-bit hosts, because `CompoundStmt` has a 32-bit field prior to its pointer-aligned trailing storage.  Can you check how memory use actually changes?  Ideally you'd be able to test it on a 32-bit host, but if you don't have one, just knowing the raw count of allocated `CompoundStmt` objects for a few large compilation benchmarks would be nice — the ideal stress test is probably something in C++ that instantiates a lot of function templates.

This is finicky, but: the `FooBitfields` structs are designed to be <= 64 bits, not just a 32-bit bitfield.  Currently, `CompoundStmt` puts the leading brace location in there just to save some space on 32-bit hosts, but it seems to me that, if we're going to put a 32-bit count somewhere, it'd be nice to put it in the `CompoundStmtBitfields` and move the leading brace location out to the main class.  I know, it's finicky.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125635/new/

https://reviews.llvm.org/D125635



More information about the cfe-commits mailing list