[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