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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

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

> In https://reviews.llvm.org/D28041#629749, @tejohnson wrote:
>
> > In https://reviews.llvm.org/D28041#629442, @mehdi_amini wrote:
> >
> > >
> >
> >
> > 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)
>
>
>
> In https://reviews.llvm.org/D28041#629835, @pcc wrote:
>
> > In https://reviews.llvm.org/D28041#629749, @tejohnson wrote:
> >
> > > In https://reviews.llvm.org/D28041#629442, @mehdi_amini wrote:
> > >
> > > >
> > >
> >
> >
> > It's far from clear to me that in assembly is the right way to go. It might be for the individual summaries but not necessarily for the combined summary as it isn't really a property of any specific module.
>
>
> I agree, but we wouldn't want two different serializations for each of the cases (knowing that the in-memory datastructure is shared).


We could just have some way to embed a YAML "blob" in the asm. Yes it would be a little ugly, but it wouldn't really matter because debugging/testing. And again: this is all predicated on assembly being the "right" format.

>> Given that, and also given that this format is for debugging/testing purposes only,
> 
> Isn't it true of the textual IR as well?

Yes, but IR in YAML format would likely be unreadable, so if you follow my point to its conclusion, YAML would be a bad choice for the IR. On the other hand, summaries are more straightforwardly representable as YAML.


https://reviews.llvm.org/D28041