This is an archive of the discontinued LLVM Phabricator instance.

[IR] Split out target specific intrinsic enums into separate headers
ClosedPublic

Authored by rnk on Dec 10 2019, 4:54 PM.

Details

Summary

This has two main effects:

  • Optimizes debug info size by saving 221.86 MB of obj file size in a Windows optimized+debug build of 'all'. This is 3.03% of 7,332.7MB of object file size.
  • Incremental step towards decoupling target intrinsics.

The enums are still compact, so adding and removing a single
target-specific intrinsic will trigger a rebuild of all of LLVM.
Assigning distinct target id spaces is potential future work.

Part of PR34259

Depends on D71318

Diff Detail

Event Timeline

rnk created this revision.Dec 10 2019, 4:54 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 10 2019, 4:54 PM
MaskRay accepted this revision.Dec 10 2019, 5:04 PM

Amazing!

llvm/utils/TableGen/IntrinsicEmitter.cpp
161

OS.indent(40 - Ints[i].EnumName.size());

166

IntrinsicPrefix.empty()

This revision is now accepted and ready to land.Dec 10 2019, 5:04 PM
efriedma added inline comments.Dec 10 2019, 5:09 PM
llvm/include/llvm/Analysis/TargetTransformInfo.h
757–760

For the sake of anyone downstream, it would be better to rename both overloads.

rnk marked an inline comment as done.Dec 10 2019, 5:21 PM
rnk added inline comments.
llvm/include/llvm/Analysis/TargetTransformInfo.h
757–760

Hm, true, when I wrote this, I think I found a way to do it such that the compiler told me about all the incorrect call sites. I forget the details, though, I did it over Thanksgiving.

echristo accepted this revision.Dec 10 2019, 7:56 PM

SGTM and thank you :)

This revision was automatically updated to reflect the committed changes.
rnk marked 2 inline comments as done.Dec 11 2019, 6:08 PM

Thanks, I split off the rename and landed it separately, since it is more mechanical.

thakis added a subscriber: thakis.Dec 11 2019, 7:25 PM

Any reason these are called .h? All our other tablegen outputs are called .inc.

rnk added a comment.Dec 12 2019, 10:30 AM

Any reason these are called .h? All our other tablegen outputs are called .inc.

Yes, they have header guards, they are not textual. Most other tablegen outputs are intended to be used with some kind of xmacro pattern, where the includer sets a macro before including the file. I could've structured things so that there is:

  • a per-target .inc file in build/include/llvm/IR/
  • a per-target .h file in llvm/include/llvm/IR, not generated

But I felt that it was less boilerplate and code to have tablegen splat out the .h file and include that from .cpp files directly.

I think this broke the reverse-iteration bot: http://lab.llvm.org:8011/builders/reverse-iteration/builds/15619

In D71320#1782204, @rnk wrote:

Any reason these are called .h? All our other tablegen outputs are called .inc.

Yes, they have header guards, they are not textual. Most other tablegen outputs are intended to be used with some kind of xmacro pattern, where the includer sets a macro before including the file. I could've structured things so that there is:

  • a per-target .inc file in build/include/llvm/IR/
  • a per-target .h file in llvm/include/llvm/IR, not generated

But I felt that it was less boilerplate and code to have tablegen splat out the .h file and include that from .cpp files directly.

Makes sense, but it's unusual.

rnk added a comment.Dec 15 2019, 12:17 PM

I think this broke the reverse-iteration bot: http://lab.llvm.org:8011/builders/reverse-iteration/builds/15619

Looks like I forgot to build polly, and @aheejin fixed it. Thanks! I usually enable polly when testing these kinds of change, but I often disable it due to warnings in isl and then forget to re-enable it.