[PATCH] D141110: [RS4GC] Remove the hardcoded GC strategy names (v2)

[Markus Böck via Phabricator via llvm-commits]

zero9178 added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:117
 
+using GCStrategyCache = std::map<StringRef, std::unique_ptr<GCStrategy>>;
+
----------------
Use of `std::map` is generally discouraged unless you require any of its properties.
As far as I can tell you're just doing pure insert and lookups on it, so you'll probably want to use `StringMap` from LLVM.



================
Comment at: llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp:3053
+
+  if (cache.count(F.getGC()) == 0) {
+    cache[F.getGC()] = getGCStrategy(F.getGC());
----------------
You are doing 3 lookups in the map here, which can be reduced to just one. 
The trick is to use `insert({F.getGC(), nullptr})`, which will return an iterator and bool as pairs. The iterator will either point to an existing entry if it was NOT inserted, or to the now inserted entry. The bool is true if inserted. 
That way you just need to check if it was inserted, if it was, assign to `second` of the entry the result of `getGCStrategy`.
At the end also just return `second` of the iterator pointing to the entry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141110