kadircet added inline comments.
================ Comment at: clang/lib/Support/RISCVVIntrinsicUtils.cpp:35 +// Concat BasicType, LMUL and Proto as key +static std::unordered_map<uint64_t, RVVType> LegalTypes; +static std::set<uint64_t> IllegalTypes; ---------------- these were previously owned by `RVVEmitter`, hence we would have one instance per invocation. After this move, and the follow up patch that introduced lazy lookup, now we actually have a race condition in tools that can perform concurrent AST builds (e.g. any tool, like clang-tidy, that might have multiple `Sema`s alive because they're being executed with `all-TU`s executor, or clangd when the user opens multiple files). Rather than having these caches as global singletons (or static local variables as things have evolved since this patch), can we: - make them owned by `RISCVIntrinsicManagerImpl` (while getting rid of the cache for TableGen purposes completely, isn't it already generating the full header, why does it need a cache?)? that way we would get one instance per `Sema` and prevent above mentioned race conditions - add some locks to synchronise access to these caches. This has been triggering crashes in clangd for a long while now (tracking it back to this issue wasn't easy, sorry for the delay) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124730/new/ https://reviews.llvm.org/D124730 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
