This is an archive of the discontinued LLVM Phabricator instance.

Optionally use nameless IR types
Needs ReviewPublic

Authored by sepavloff on Feb 26 2018, 8:31 PM.

Details

Reviewers
rsmith
rjmccall
Summary

Type in the LLVM IR may have names but only for the purpose of human
readability (see discussions in https://reviews.llvm.org/D40567,
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20171127/210816.html
and http://lists.llvm.org/pipermail/llvm-dev/2017-December/119585.html).
In the case when the resulting IR is not proposed for reading, for
instance, when compilation produces machine code, the type names are waste
of memory. In some cases, when types are nested in other types, the memory
expenses may be really large.

This change implements new clang option, '--ir-type-names=', which controls
if IR types should be given human readable names. The option may have
values 'use' or 'none', which turn names on or off correspondently. If no such
option was specified, compiler assign names when output may be read by a
human, namely when IR is saved beyond compilation or in debug builds.

Diff Detail

Event Timeline

sepavloff created this revision.Feb 26 2018, 8:31 PM
rjmccall added inline comments.Feb 26 2018, 11:49 PM
include/clang/Driver/Options.td
1740

This is an unusual spelling for the option in a number of ways:

  • We generally don't use -- options; I think all the ones we have are strictly for legacy support.
  • A lot of similar options are in the -f or -m namespaces, although that's not as consistent and we could reasonably make this an exception.
  • -foo=bar options are generally used for options that are expected to take a variety of different values; this seems basically boolean. Are you expecting future growth here?
sepavloff updated this revision to Diff 136087.Feb 27 2018, 8:53 AM

Updated patch

  • command line option was renamed to -fir-type-names=,
  • this option is not hidden, as most options in -f namespace,
  • new value, auto was added to possible values of this option.
sepavloff marked an inline comment as done.Feb 27 2018, 9:02 AM
sepavloff added inline comments.
include/clang/Driver/Options.td
1740

The option is in fact a three-state one, the third 'value' is absence of the option. In this case the option value is calculated from the type of action (produce ll file or not) and from the type of build (in debug builds types are named by default). To avoid misunderstanding I added new value, 'auto' for this purpose.

The option was renamed to -fir-type-names= and it is not hidden anymore.

pcc added a subscriber: pcc.Mar 6 2018, 8:19 PM

Why is this a driver flag? This seems like it ought to be a cc1-only flag to me.

rsmith added inline comments.Mar 6 2018, 8:24 PM
include/clang/Driver/Options.td
644

Having "none" and "use" as values for the same option seems like a category error. always / never / auto would make some sense, though.

sepavloff marked an inline comment as done.Mar 6 2018, 8:31 PM
In D43805#1029455, @pcc wrote:

Why is this a driver flag? This seems like it ought to be a cc1-only flag to me.

Yous are right, this is more like development option. But having the driver flag allows using compiler in builds with either option.
The option was hidden, probably it should be make hidden again.
If the backend will be changed so that it will not depend on IR type names, the default mode can be set to nameless types and the option can become -cc1 only.

sepavloff added inline comments.Mar 6 2018, 8:34 PM
include/clang/Driver/Options.td
644

Indeed. Will change it.

pcc added a comment.Mar 6 2018, 8:59 PM

If the backend will be changed so that it will not depend on IR type names

Are you referring to D43199? If so it seems to me that this should be a cc1 flag that defaults to whether -flto=thin is passed. In any case it seems like a bad idea to deliberately generate different code depending on whether we were compiled with NDEBUG.

In D43805#1029479, @pcc wrote:

If the backend will be changed so that it will not depend on IR type names

Are you referring to D43199? If so it seems to me that this should be a cc1 flag that defaults to whether -flto=thin is passed. In any case it seems like a bad idea to deliberately generate different code depending on whether we were compiled with NDEBUG.

No, I try to implement alternative approach, to solve the problem targeted in D40508. If IR type names are only for human readability, than using them in opaque type resolution does not look reasonable. Probably, more correct way is to make type merge only as a side effect of merge of other entities, which may have "real" names, that is functions and variables.

sepavloff updated this revision to Diff 137336.Mar 7 2018, 1:39 AM

Use more consistent option names

Any feedback?

Thanks,
--Serge