[PATCH] D142314: [ORC] Add a NonOwningSymbolStringPtr utility.

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 31 13:41:26 PST 2023


dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Sure, looks good



================
Comment at: llvm/include/llvm/ExecutionEngine/Orc/SymbolStringPool.h:87-88
+
+  friend bool operator!=(const SymbolStringPtrBase &LHS,
+                         const SymbolStringPtrBase &RHS) {
+    return !(LHS == RHS);
----------------
lhames wrote:
> dblaikie wrote:
> > If these things are only `sizeof(void*)` then maybe pass by value, rather than const ref?
> They could be pass by value. Is there likely to be any benefit on a trivially inline-able method like this? Or is it mostly a style issue? 
general guideline - maybe makes -O0 builds slightly less bad


================
Comment at: llvm/unittests/ExecutionEngine/Orc/SymbolStringPoolTest.cpp:20-22
+  size_t getRefCount(const SymbolStringPtrBase &S) const {
+    return SP.getRefCount(S);
+  }
----------------
Awkward to have to wrap the API like this - but I guess the inconvenience of having to touch the class definition for every test to do this http://google.github.io/googletest/reference/testing.html#FRIEND_TEST is annoying too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142314



More information about the llvm-commits mailing list