[PATCH] D127778: [LTO][ELF] Add selective --save-temps= option

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Config.h:216
   bool saveTemps;
+  llvm::SmallSet<std::string, 9> saveTempsArgs;
   std::vector<std::pair<llvm::GlobPattern, uint32_t>> shuffleSections;
----------------
use 0.

this isn't a common option. having a large size makes accessing other bool variables slow.


================
Comment at: lld/test/ELF/lto/save-temps-eq.ll:2
+; REQUIRES: x86
+; RUN: rm -fr %t* && mkdir %t && cd %t
+
----------------
Using glob with `rm` is dangerous.


================
Comment at: lld/test/ELF/lto/save-temps-eq.ll:13
+; RUN: ld.lld main.o thin1.o thin2.o --save-temps
+; RUN: mv `find . ! -name main\.o ! -name thin1\.o ! -name thin2\.o ! -name \.` %t.all
+
----------------
You can use ls with glob. `find` in lit is very unusual.


================
Comment at: lld/test/ELF/lto/save-temps-eq.ll:100
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
----------------
use a normal x86_64 triple.


================
Comment at: llvm/lib/LTO/LTOBackend.cpp:132
 
-  setHook("0.preopt", PreOptModuleHook);
-  setHook("1.promote", PostPromoteModuleHook);
-  setHook("2.internalize", PostInternalizeModuleHook);
-  setHook("3.import", PostImportModuleHook);
-  setHook("4.opt", PostOptModuleHook);
-  setHook("5.precodegen", PreCodeGenModuleHook);
-
-  CombinedIndexHook =
+  auto _CombinedIndexHook =
       [=](const ModuleSummaryIndex &Index,
----------------
Avoid a reserved identifier. https://en.cppreference.com/w/cpp/language/identifiers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127778