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

2018-02-26 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff abandoned this revision. sepavloff added a comment. Using llvm type names is considered a wrong direction. https://reviews.llvm.org/D40508 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

2017-11-29 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Are you trying to use LLVM struct type identity to infer information about the source program? That is not and has never been a guarantee. https://reviews.llvm.org/D40508 ___ cfe-commits mailing list

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

2017-11-29 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment. In https://reviews.llvm.org/D40508#939513, @rjmccall wrote: > In https://reviews.llvm.org/D40508#938854, @sepavloff wrote: > > > In https://reviews.llvm.org/D40508#938686, @rjmccall wrote: > > > > > The LLVM linking model does not actually depend on struct type names

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

2017-11-29 Thread John McCall via Phabricator via cfe-commits
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: > > > > >

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

2017-11-29 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment. 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

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

2017-11-28 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. 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

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

2017-11-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment. 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

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

2017-11-28 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. 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

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

2017-11-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment. https://reviews.llvm.org/D40567 presents a patch that adds template argument names to class template specializations. It also describes the use case in which type names are used to identify type across different TUs. Adding template parameters obviously increase

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

2017-11-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment. In https://reviews.llvm.org/D40508#937212, @rjmccall wrote: > In Swift's IRGen, we have an option controlling whether to emit meaningful > value names. That option can be set directly from the command line, but it > defaults to whether we're emitting IR assembly

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

2017-11-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff marked 3 inline comments as done. sepavloff added a comment. In https://reviews.llvm.org/D40508#936617, @rnk wrote: > It's not clear to me that this abbreviation functionality should live in > Support. Clang probably wants enough control over this (assuming we're doing > it) that the

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

2017-11-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff updated this revision to Diff 124587. sepavloff added a comment. Updated patch as part of it was committed in https://reviews.llvm.org/rL319178 https://reviews.llvm.org/D40508 Files: include/clang/AST/PrettyPrinter.h lib/AST/TypePrinter.cpp lib/CodeGen/CodeGenTypes.cpp

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

2017-11-27 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In Swift's IRGen, we have an option controlling whether to emit meaningful value names. That option can be set directly from the command line, but it defaults to whether we're emitting IR assembly (i.e. .ll, not .bc), which means that the clients most interested in

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

2017-11-27 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. It's not clear to me that this abbreviation functionality should live in Support. Clang probably wants enough control over this (assuming we're doing it) that the logic should live in clang. I also think we might want to try solving this a completely different way: just

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

2017-11-27 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff created this revision. Herald added a subscriber: JDevlieghere. If a source file extensively uses templates, resulting LLVM IR may have types with huge names. It may occur if a record type is defined in a class. In this case its type name contains all declaration contexts and, if a