[PATCH] D132988: Changes to use a string pool for symbols
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 8 12:19:45 PDT 2022
dblaikie added inline comments.
================
Comment at: llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h:418
static Symbol &constructCommon(BumpPtrAllocator &Allocator, Block &Base,
- StringRef Name, orc::ExecutorAddrDiff Size,
- Scope S, bool IsLive) {
- assert(!Name.empty() && "Common symbol name cannot be empty");
+ orc::SymbolStringPtr &&Name,
+ orc::ExecutorAddrDiff Size, Scope S,
----------------
jaredwy wrote:
> sgraenitz wrote:
> > Is there a special reason for these methods to take RValue references? Otherwise, these look like classical sink arguments that should be moved in and taken by value (just remove the `&&`).
> No not really and in fact started life that way, just was looking to avoid the overhead of the add/decrement of the ownership counter when I noticed every usage was taking ownership, will remove :)
If you std::move at the caller and in the callee, you shouldn't need to add/decrement ownership counters even when passing by value. (it'll add an extra move, but no copies - copies are what cause add/decrement), so this should be good
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132988/new/
https://reviews.llvm.org/D132988
More information about the llvm-commits
mailing list