[PATCH] D33615: Move summary dead stripping before regular LTO and record results in the combined summary

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 1 13:44:54 PDT 2017


pcc added a comment.

It looks like you



================
Comment at: include/llvm/IR/ModuleSummaryIndexYAML.h:132
+  unsigned Linkage;
+  bool NotEligibleToImport, Live;
   std::vector<uint64_t> TypeTests;
----------------
mehdi_amini wrote:
> The addition of Linkage and NotEligibleToImport are unrelated to this change right? I feel you could commit them separately now.
> (even the serialization of "Live" here isn't clear to me if it isn't something that could just be split out).
Would we be using Linkage and NotEligibleToImport for anything though? It seems a little odd to add fields here without any test coverage. (That's not to say that we shouldn't add the fields eventually, but I think it should be accompanied by tests, i.e. we should add a JSON summary dumper to llvm-lto2 or something and convert some tests over to use it.)

I think the serialization of Live was part of D33702, and I'd prefer it to be done as part of that change.


Repository:
  rL LLVM

https://reviews.llvm.org/D33615





More information about the llvm-commits mailing list