[PATCH] D110196: [IR] Modernize and Cleanup LLVM/IR folder
Duncan P. N. Exon Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 12 13:12:31 PST 2021
dexonsmith added a comment.
Just a couple of comments inline to keep in mind if/when you start splitting pieces of this out.
================
Comment at: llvm/lib/IR/Function.cpp:692-695
+void Function::setGC(const std::string &Str) {
setValueSubclassDataBit(14, !Str.empty());
getContext().setGC(*this, std::move(Str));
}
----------------
Is this an improvement? With this change (and the one to `LLVMContext::setGC()`), there's potentially allocation traffic with this usage pattern:
```
lang=c++
std::string GC = /* ... */;
F.setGC(std::move(GC));
```
where previously there was none.
Another option might be to make this (and the called API) a `StringRef`, and audit callers to delay any std::string allocation until `LLVMContext::setGC()`.
================
Comment at: llvm/lib/IR/IRPrintingPasses.cpp:55-56
PrintFunctionPass::PrintFunctionPass() : OS(dbgs()) {}
-PrintFunctionPass::PrintFunctionPass(raw_ostream &OS, const std::string &Banner)
- : OS(OS), Banner(Banner) {}
+PrintFunctionPass::PrintFunctionPass(raw_ostream &OS, std::string Banner)
+ : OS(OS), Banner(std::move(Banner)) {}
----------------
When you split this out, I'd suggest using `StringRef` as the parameter and then `Banner(Banner.str())`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110196/new/
https://reviews.llvm.org/D110196
More information about the llvm-commits
mailing list