AaronBallman wrote: > > > @AaronBallman See test results from compile-time-tracker here: > > > https://llvm-compile-time-tracker.com/compare.php?from=693a458287d019c5c6a66fe3019d099df2978cdb&to=dbb5e29d136a18060ba6759b328ad80fa9cea649. > > > It looks like that there is a statistically meaningful difference, but > > > it's only about 0.05..0.25% depending on the test. Is it considered OK? > > > > > > Yeah, this seems to have noticeable impact on compile times for every > > compilation; out of curiosity, have you tried an approach where this > > information is stored in `ExtQuals` instead? That's heap allocated, but > > would mean that the only folks paying the cost are the ones using the > > functionality. > > `Qualifiers` is an inline value type representing all possible qualifiers, > separated from its application to any specific type. `ExtQuals` represents an > application of qualifiers that don't fit into the inline fast-qualifiers bits > to a specific type. `ExtQuals` stores a `Qualifiers` inline, with a > precondition that the fast qualifier bits are clear. Outside of that, we > never store `Qualifiers` long-term AFAIK. > > `PointerAuthQualifier` is 32 bits. Adding it to `Qualifiers` increases > `Qualifiers` from 32 bits (mostly occupied) to 64 bits. `__ptrauth` > qualifiers are not a fast qualifier, so when applied to a type, they require > the use of an `ExtQuals` node. > > Given all that, I'm not sure what you're asking for. Storing uncommon > qualifiers out of line is what we already do with `QualType` and is why > `ExtQuals` exists; doing it again with `Qualifiers` doesn't seem to serve any > purpose. It's certainly not going to make `Qualifiers` smaller or more > efficient to work with, since `PointerAuthQualifier` is smaller than a > pointer. `ExtQuals` is 128-bit-aligned and starts with two pointers, so > there's space for 64 bits of qualifiers on 32-bit hosts and 128 bits of > qualifiers on 64-bit hosts before `ExtQuals` grows.
Ah, okay, thank you! I had two concerns, but I think neither are valid now that I better understand how `ExtQuals` works: 1) I thought we were increasing the in-memory size of `Qualifiers` in a way that matter (like `SplitQualType`, `ExtProtoInfo` primarily), 2) I thought we had 32-bit builds of Clang so all the places where we pass/return a `Qualifiers` would require passing in multiple registers now. > The overhead is probably from additional checks rather than any cost > associated with working with a 64-bit `Qualifiers` value. We could look into > ways to optimize those checks (e.g. qualifier compatibility) in the common > cases where there are no extended qualifiers. It's also possible that merging > `PointerAuthQualifier` into `Mask` inside `Qualifiers` would make some of the > low-level handling more efficient. Do you think the performance concerns are sufficient to warrant doing this optimization work up front? .25% is big enough to warrant concern, but not big enough to need to ask for onerous efforts. https://github.com/llvm/llvm-project/pull/84384 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
