[PATCH] D28041: IR: Module summary representation for type identifiers; summary test scaffolding for lowertypetests.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D28041#629442, @mehdi_amini wrote:

> I don't have any strong feeling against YAML, but we should take this decision for the full summary and not just for the "type test", so I'd like to hear from Teresa about that (in the past we were considering a LLVM "metadata" serialization instead, or similar, so that it can round-trip in the IR itself).


It would be optimal to have a serialization in the LLVM assembly format (I started to look into doing that some time ago but got side tracked - I was thinking not as metadata since we don't carry it internally as metadata, but a new format e.g. similar to the way attributes are emitted as a separate format not metadata). Rather than taking the trouble to emit as YAML, why not emit into the LLVM assembly from the start?

> (I'm tired of the llvm-bcanalyzer tests for the summary, so any kind of textual serialization of the summaries for testing and debugging purpose will be welcome at this point)



================
Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:401
+  // FIXME: Add bitcode read/write support for this field.
+  std::map<std::string, TypeIdSummary> TypeIdMap;
+
----------------
mehdi_amini wrote:
> Is this supposed to be "per module" or should it end up global to the index? (My understanding was the latter but you implemented the former AFAICT).
Mehdi: What makes this per-module vs global? The same ModuleSummaryIndex data structure is used for both the permodule index and the combined index.

pcc: Why no accessors for this map?


https://reviews.llvm.org/D28041