[PATCH] D40508: Replace long type names in IR with hashes

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 29 12:08:11 PST 2017


rjmccall added a comment.

In https://reviews.llvm.org/D40508#938854, @sepavloff wrote:

> In https://reviews.llvm.org/D40508#938686, @rjmccall wrote:
>
> > In https://reviews.llvm.org/D40508#938675, @sepavloff wrote:
> >
> > > In https://reviews.llvm.org/D40508#938040, @rjmccall wrote:
> > >
> > > > My skepticism about the raw_ostream is not about the design of having a custom raw_ostream subclass, it's that that subclass could conceivably be re-used by some other client.  It feels like it belongs as an internal hack in Clang absent some real evidence that someone else would use it.
> > >
> > >
> > > This class can be helpful in various cases where string identifier must persistently identify an object and memory consumption must be low. It may be:
> > >
> > > - If we introduce an option that allows a user to specify how many symbols of full type name are kept in abbreviated name, then `llvm-link` may see types named as `foo<int>` and `2544896211ff669ed44dccd265f4c9163b340190`, if they come from modules compiled with different options. To find out that these are the same type, it must have access to the same machinery.
> >
> >
> > The LLVM linking model does not actually depend on struct type names matching.  My understanding is that, at best, it uses that as a heuristic for deciding whether to make an effort to unify two types, but it's not something that's ultimately supposed to impact IR semantics.
>
>
> It is mainly true with an exception, when `llvm-link` resolves opaque types it relies on type name only. And this behavior creates the issue that https://reviews.llvm.org/D40567 tries to resolve.


It is not clear from that report what the actual problem is.  Two incomplete types get merged by the linker?  So what?

>> If we needed IR type names to match reliably, we would use a mangled name, not a pretty-print.
> 
> There is no requirement for IR type name to be an identifier, so pretty-print fits the need of type identification.

Not really; pretty-printing drops a lot of information that is pertinent in a stable identifier, like scopes and so on, and makes arbitrary decisions about other things, like where to insert spaces, namespace qualifiers, etc.


https://reviews.llvm.org/D40508





More information about the cfe-commits mailing list