[PATCH] D94486: [LTO] Expose opt() in LTOBackend (NFC).
Duncan P. N. Exon Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 12 16:38:34 PST 2021
dexonsmith added inline comments.
================
Comment at: llvm/lib/LTO/LTOBackend.cpp:486
+bool lto::opt(const Config &Conf, TargetMachine *TM, unsigned Task, Module &Mod,
+ bool IsThinLTO, ModuleSummaryIndex *ExportSummary,
----------------
fhahn wrote:
> steven_wu wrote:
> > Any reason why the function need to be moved instead of just renamed? I will reduce the diff if possible.
> Unfortunately it is originally defined in a anonymous namespace, so I think it has to be moved out of that to be defined in `lto::`.
[drive-by comments]
I think just adding the `lto::` should do the right thing. If not, you can temporarily close the anonymous namespace and reopen it.
Another option is an NFC prep commit that limits the anonymous namespace to `.cpp`-declared structures, and adds a `static` keyword on each function that's no longer "anonymous". That'd match the more typical LLVM style... reducing this patch to `s/static bool opt/bool lto::opt/`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94486/new/
https://reviews.llvm.org/D94486
More information about the llvm-commits
mailing list