[PATCH] D20268: Resolution-based LTO API.

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 18:54:28 PDT 2016


mehdi_amini added inline comments.

================
Comment at: include/llvm/LTO/LTO.h:126
@@ +125,3 @@
+        return true;
+      return Flags & object::BasicSymbolRef::SF_FormatSpecific;
+    }
----------------
What about 

```
return Flags & object::BasicSymbolRef::SF_Global ||
          Flags & object::BasicSymbolRef::SF_FormatSpecific;
```

================
Comment at: lib/LTO/LTO.cpp:281
@@ +280,3 @@
+    SymbolResolution Res = *ResI++;
+    addSymbolToGlobalRes(Obj.get(), Used, Sym, Res, 0);
+
----------------
This is marked done, but doesn't seem to be?

================
Comment at: lib/LTO/LTOBackend.cpp:59
@@ +58,3 @@
+          PathPrefix += "." + utostr(Task);
+      } else
+        PathPrefix = M.getModuleIdentifier();
----------------
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.
```


https://reviews.llvm.org/D20268





More information about the llvm-commits mailing list