[PATCH] D124730: [RISCV][NFC] Refactor RISC-V vector intrinsic utils.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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