[clang] [NFC] [clang] Use std::string instead of StringRef to reduce stack usage (PR #114285)
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 30 12:47:12 PDT 2024
erichkeane wrote:
> > If you really want to work around MSVC's bug, you could change tablegen to emit the string literals as constant globals / StringLiteral
>
> I spent a bit thinking about this, and this function is incredibly wasteful. It ends up re-doing a ton of work that is ALREADY done. It was convenient at the time to write it that way, but with a little intelligence at tablegen, we can simplify this function a million-times.
>
> Some things we can do (and should do all): 1- We can calculate the scope, so we don't actually compare those. We're checking against "clang" and "" constantly, lets pre-calculate that into some sort of enum. There are only so many valid values here (and, IIRC, this has already been checked to be valid), so we can switch on those easily. PERHAPS another tablegen file to make sure we get all the normalized ones, but pretty trivial.
>
> 2- we could reduce the number of comparisons against the name: At that point, it ends up being:
>
> ```
> if (Name == "aarch64_sve_pcs") {
> if (getSyntax() == AttributeCommonInfo::AS_GNU && calculatedScope == Scope::None)
> return 0;
> if (getSyntax() == AttributeCommonInfo::AS_CXX11 && calculatedScope == Scope::Clang)
> return 1;
> if (getSyntax() == AttributeCommonInfo::AS_C23 && calculatedScope == Clang)
> return 2;
> ```
>
> 3- STOP being dummies and re-doing all the work that the `trie` in getAttrKind already did! We already know that the spelling is one-of-a-couple for each of the variants, so the actual 'does this string match this other one' is useless, we just need to know which it is in a list. MOST (including the one in the example above), all are the same, so those comparisons are dumb/wasted. Lets see if we can remove those.
>
> For cases where there are multiple spellings, we can compare length! For example: 1st I found where spelling matters, is `AT_ArgumentWithTypeTag:`, which is either `argument_with_type_tag` or `pointer_with_type_tag`. At THAT point, we could just do `Name.length() == 22` and `Name.length() == 21`.
>
> We already know it is ONE of those two, and just need to figure out which.
>
> IN SUMMARY: I am convinced somewhat we can just teach tablegen to be 'smarter' about this file and save ourselves ~100% of those string compares and replace them with numeric comparisons, or enum comparisons.
>
> The result is something that is: A- acceptable to the community/maintainers B- sidesteps/never has this problem again for MSVC C- Improves performance.
BTW, @chinmaydd : Please let me know if this is something you're able to do. I might be able to free up some cycles later this week/next week to put effort into this if not.
https://github.com/llvm/llvm-project/pull/114285
More information about the cfe-commits
mailing list