[PATCH] D20268: Resolution-based LTO API.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: lib/LTO/LTO.cpp:281
@@ +280,3 @@
+    SymbolResolution Res = *ResI++;
+    addSymbolToGlobalRes(Obj.get(), Used, Sym, Res, 0);
+
----------------
mehdi_amini wrote:
> This is marked done, but doesn't seem to be?
See the first three lines of `LTO::run`.

================
Comment at: lib/LTO/LTOBackend.cpp:59
@@ +58,3 @@
+          PathPrefix += "." + utostr(Task);
+      } else
+        PathPrefix = M.getModuleIdentifier();
----------------
mehdi_amini wrote:
> Deriving an output temp file from a user-supplied *output* is fine, what does not make sense to me is to write stuff where the *input* files are. This is unlike what we usually do (the static archives could be in a read-only system directory for instance, this is quite common on our setup).
> 
> You can maintain whatever behavior Gold currently has by passing an option to `addSaveTemps` maybe. Right now the API being `addSaveTemps(std::string OutputFileName)` I don't expect files to be written anywhere unexpected.
> 
> Also the doxygen is pretty clear about what should be expected from this API:
> 
> ```
>   /// This is a convenience function that configures this Config object to write
>   /// temporary files named after the given OutputFileName for each of the LTO
>   /// phases to disk. A client can use this function to implement -save-temps.
> ```
I agree with you that we should move to a temp file naming scheme based on output files, however like Teresa I think that should be discussed separately from this patch. I don't think we need a flag either, whatever we decide to do here should apply to all linkers.

Added a FIXME here to change the naming scheme.


https://reviews.llvm.org/D20268