Commit Graph

67 Commits

Author SHA1 Message Date
Peter Collingbourne d4135bbc30 DebugInfo: New metadata representation for global variables.
This patch reverses the edge from DIGlobalVariable to GlobalVariable.
This will allow us to more easily preserve debug info metadata when
manipulating global variables.

Fixes PR30362. A program for upgrading test cases is attached to that
bug.

Differential Revision: http://reviews.llvm.org/D20147

llvm-svn: 281284
2016-09-13 01:12:59 +00:00
Mehdi Amini ebb3434850 Fix ThinLTO crash with debug info
Because the recent change about ODR type uniquing in the context,
we can reach types defined in another module during IR linking.
This triggered some assertions in case we IR link without starting
from an empty module. To alleviate that, we can self-map metadata
defined in the destination module so that they won't be visited.

Differential Revision: https://reviews.llvm.org/D23841

llvm-svn: 280599
2016-09-03 21:12:33 +00:00
Justin Bogner 375f71e3a3 Linker: Avoid some ridiculous indentation by using a temporary. NFC
This was indented really awkwardly, and clang-format didn't seem to
know how to do any better. Avoid the issue with a temporary variable.

llvm-svn: 278756
2016-08-15 22:41:42 +00:00
David Majnemer c700490f48 Use the range variant of remove_if instead of unpacking begin/end
No functionality change is intended.

llvm-svn: 278475
2016-08-12 04:32:37 +00:00
Peter Collingbourne 7efd750607 IR: New representation for CFI and virtual call optimization pass metadata.
The bitset metadata currently used in LLVM has a few problems:

1. It has the wrong name. The name "bitset" refers to an implementation
   detail of one use of the metadata (i.e. its original use case, CFI).
   This makes it harder to understand, as the name makes no sense in the
   context of virtual call optimization.

2. It is represented using a global named metadata node, rather than
   being directly associated with a global. This makes it harder to
   manipulate the metadata when rebuilding global variables, summarise it
   as part of ThinLTO and drop unused metadata when associated globals are
   dropped. For this reason, CFI does not currently work correctly when
   both CFI and vcall opt are enabled, as vcall opt needs to rebuild vtable
   globals, and fails to associate metadata with the rebuilt globals. As I
   understand it, the same problem could also affect ASan, which rebuilds
   globals with a red zone.

This patch solves both of those problems in the following way:

1. Rename the metadata to "type metadata". This new name reflects how
   the metadata is currently being used (i.e. to represent type information
   for CFI and vtable opt). The new name is reflected in the name for the
   associated intrinsic (llvm.type.test) and pass (LowerTypeTests).

2. Attach metadata directly to the globals that it pertains to, rather
   than using the "llvm.bitsets" global metadata node as we are doing now.
   This is done using the newly introduced capability to attach
   metadata to global variables (r271348 and r271358).

See also: http://lists.llvm.org/pipermail/llvm-dev/2016-June/100462.html

Differential Revision: http://reviews.llvm.org/D21053

llvm-svn: 273729
2016-06-24 21:21:32 +00:00
Peter Collingbourne 4f7c16dd53 Linker: Copy metadata when linking declarations.
Differential Revision: http://reviews.llvm.org/D21624

llvm-svn: 273692
2016-06-24 17:42:21 +00:00
Artur Pilipenko 6c7a8abf5c Remangle intrinsics names when types are renamed
This is a resubmittion of previously reverted rL273568.

This is a fix for the problem mentioned in "LTO and intrinsics mangling" llvm-dev mail thread:
http://lists.llvm.org/pipermail/llvm-dev/2016-April/098387.html

Reviewers: mehdi_amini, reames

Differential Revision: http://reviews.llvm.org/D19373

llvm-svn: 273686
2016-06-24 15:10:29 +00:00
Hans Wennborg a63b50afb8 Revert r273568 "Remangle intrinsics names when types are renamed"
It broke 2008-07-15-Bswap.ll and 2009-09-01-PostRAProlog.ll

llvm-svn: 273574
2016-06-23 16:13:23 +00:00
Artur Pilipenko f0c9f81379 Remangle intrinsics names when types are renamed
This is a fix for the problem mentioned in "LTO and intrinsics mangling" llvm-dev mail thread:
http://lists.llvm.org/pipermail/llvm-dev/2016-April/098387.html

Reviewers: mehdi_amini, reames

Differential Revision: http://reviews.llvm.org/D19373

llvm-svn: 273568
2016-06-23 15:25:09 +00:00
Justin Lebar c05f3c9942 [IR] [DAE] Copy comdats during DAE, and don't copy comdats in GlobalObject::copyAttributesFrom.
Summary: This reverts the changes to Globals.cpp and IRMover.cpp in
"[IR] Copy comdats in GlobalObject::copyAttributesFrom" (D20631,
rL270743).

The DeadArgElim test is left unchanged, and we change DAE to explicitly
copy comdats.

The reverted change breaks copyAttributesFrom when the destination lives
in a different module from the source.  The decision in D21255 was to
revert this patch and handle comdat copying separately from
copyAttributesFrom.

Reviewers: majnemer, rnk

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D21403

llvm-svn: 272855
2016-06-15 23:20:15 +00:00
Peter Collingbourne 96efdd6107 IR: Introduce local_unnamed_addr attribute.
If a local_unnamed_addr attribute is attached to a global, the address
is known to be insignificant within the module. It is distinct from the
existing unnamed_addr attribute in that it only describes a local property
of the module rather than a global property of the symbol.

This attribute is intended to be used by the code generator and LTO to allow
the linker to decide whether the global needs to be in the symbol table. It is
possible to exclude a global from the symbol table if three things are true:
- This attribute is present on every instance of the global (which means that
  the normal rule that the global must have a unique address can be broken without
  being observable by the program by performing comparisons against the global's
  address)
- The global has linkonce_odr linkage (which means that each linkage unit must have
  its own copy of the global if it requires one, and the copy in each linkage unit
  must be the same)
- It is a constant or a function (which means that the program cannot observe that
  the unique-address rule has been broken by writing to the global)

Although this attribute could in principle be computed from the module
contents, LTO clients (i.e. linkers) will normally need to be able to compute
this property as part of symbol resolution, and it would be inefficient to
materialize every module just to compute it.

See:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160509/356401.html
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160516/356738.html
for earlier discussion.

Part of the fix for PR27553.

Differential Revision: http://reviews.llvm.org/D20348

llvm-svn: 272709
2016-06-14 21:01:22 +00:00
Benjamin Kramer d3f4c05aea Move instances of std::function.
Or replace with llvm::function_ref if it's never stored. NFC intended.

llvm-svn: 272513
2016-06-12 16:13:55 +00:00
Davide Italiano 953396536b [Linker/IRMover] Simplify the code a bit. NFCI.
llvm-svn: 272013
2016-06-07 14:55:04 +00:00
Benjamin Kramer 82de7d323d Apply clang-tidy's misc-move-constructor-init throughout LLVM.
No functionality change intended, maybe a tiny performance improvement.

llvm-svn: 270997
2016-05-27 14:27:24 +00:00
Peter Collingbourne 1eaa97f439 Linker: teach the IR mover to return llvm::Error.
This will be needed in order to consistently return an Error
to clients of the API being developed in D20268.

Differential Revision: http://reviews.llvm.org/D20550

llvm-svn: 270967
2016-05-27 05:21:35 +00:00
Mehdi Amini cc8c107e6a ValueMaterializer: rename materializeDeclFor() to materialize()
It may materialize a declaration, or a definition. The name could
be misleading. This is following a merge of materializeInitFor()
into materializeDeclFor().

Differential Revision: http://reviews.llvm.org/D20593

llvm-svn: 270759
2016-05-25 21:03:21 +00:00
Mehdi Amini 53a6672e21 ValueMaterializer: fuse materializeDeclFor and materializeInitFor (NFC)
They were originally separated to handle the co-recursion between
the ValueMapper and the ValueMaterializer. This recursion does not
exist anymore: the ValueMapper now uses a Worklist and the
ValueMaterializer is scheduling job on the Worklist.

Differential Revision: http://reviews.llvm.org/D20593

From: Mehdi Amini <mehdi.amini@apple.com>
llvm-svn: 270758
2016-05-25 21:01:51 +00:00
Mehdi Amini 3d4f3a0da9 IRLinker: fix double scheduling of mapping a global value because of an alias
This test was hitting an assertion in the value mapper because
the IRLinker was trying to map two times @A while materializing
the initializer for @C.

Fix http://llvm.org/PR27850

Differential Revision: http://reviews.llvm.org/D20586

llvm-svn: 270757
2016-05-25 21:00:44 +00:00
Reid Kleckner c0a0363d5c [IR] Copy comdats in GlobalObject::copyAttributesFrom
This is probably correct for all uses except cross-module IR linking,
where we need to move the comdat from the source module to the
destination module.

Fixes PR27870.

Reviewers: majnemer

Differential Revision: http://reviews.llvm.org/D20631

llvm-svn: 270743
2016-05-25 18:36:22 +00:00
Rafael Espindola 83658d6e7a Return a StringRef from getSection.
This is similar to how getName is handled.

llvm-svn: 269218
2016-05-11 18:21:59 +00:00
Rafael Espindola 15ca14c0b9 Fix recursive -only-needed.
We were assuming that only linkonce_odr GVs were lazy linked.

llvm-svn: 266995
2016-04-21 14:56:33 +00:00
Duncan P. N. Exon Smith a4810fac19 Linker: Avoid constructing ValueMap::MDMapT
Calling ValueMap::MD lazily constructs a ValueMap, which mallocs the
buckets.  Instead of swapping constructed maps, move around the
underlying Optional<MDMapT>.  This gets rid of some unnecessary malloc
traffic from r266579 (not that it showed up on a profile).

llvm-svn: 266761
2016-04-19 16:57:24 +00:00
Mehdi Amini 113adde594 ModuleLinker: Do not import linkonce/weak as "external_weak"
Summary:
There is no reason to have a weak reference because the external
definition will be weak.

Reviewers: rafael

Subscribers: llvm-commits, tejohnson

Differential Revision: http://reviews.llvm.org/D19267

From: Mehdi Amini <mehdi.amini@apple.com>
llvm-svn: 266750
2016-04-19 16:11:05 +00:00
Duncan P. N. Exon Smith 565a0aa1c4 Linker: Share a single Metadata map for the lifetime of IRMover
Cache the result of mapping metadata nodes between instances of IRLinker
(i.e., for the lifetime of IRMover).  There shouldn't be any real
functional change here, but this should give a major speedup.  I had
loaned this to Mehdi when he tested performance of r266446, and the two
patches together gave a 10x speedup in metadata mapping.

llvm-svn: 266579
2016-04-17 23:30:31 +00:00
Duncan P. N. Exon Smith 0fdaf8c9c2 Linker: Don't double-schedule appending variables
Add an assertion to ValueMapper that prevents double-scheduling of
GlobalValues to remap, and fix the one place it happened.  There are
tons of tests that fail with this assertion in place and without the
code change, so I'm not adding another.

Although it looks related, r266563 was, indeed, removing dead code.
AFAICT, this cross-file double-scheduling started in r266510 when the
cross-file recursion was removed.

llvm-svn: 266569
2016-04-17 19:40:20 +00:00
Duncan P. N. Exon Smith d7c560e166 IRMover: Remove dead code, NFC
llvm-svn: 266563
2016-04-17 18:21:47 +00:00
Duncan P. N. Exon Smith 39423b0294 Reapply "ValueMapper: Eliminate cross-file co-recursion, NFC"
This reverts commit r266507, reapplying r266503 (and r266505
"ValueMapper: Use API from r266503 in unit tests, NFC") completely
unchanged.

I reverted because of a bot failure here:
  http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/16810/

However, looking more closely, the failure was from a host-compiler
crash (clang 3.7.1) when building:
  lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/DwarfAccelTable.cpp.o

I didn't modify that file, or anything it includes, with that commit.

The next build (which hadn't picked up my revert) got past it:
  http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/16811/

I think this was just unfortunate timing.  I suppose the bot must be
flakey.

llvm-svn: 266510
2016-04-16 02:29:55 +00:00
Duncan P. N. Exon Smith 6fe1ff260b Revert "ValueMapper: Eliminate cross-file co-recursion, NFC"
This reverts commit r266503, in case it's the root cause of this bot
failure:

  http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/16810

I'm also reverting r266505 -- "ValueMapper: Use API from r266503 in unit
tests, NFC" -- since it's in the way.

llvm-svn: 266507
2016-04-16 02:05:33 +00:00
Duncan P. N. Exon Smith f0d73f95c1 ValueMapper: Eliminate cross-file co-recursion, NFC
Eliminate co-recursion of Mapper::mapValue through
ValueMaterializer::materializeInitFor, through a major redesign of the
ValueMapper.cpp interface.

  - Expose a ValueMapper class that controls the entry points to the
    mapping algorithms.
  - Change IRLinker to use ValueMapper directly, rather than
    llvm::RemapInstruction, llvm::MapValue, etc.
  - Use (e.g.) ValueMapper::scheduleMapGlobalInit to add mapping work to
    a worklist in ValueMapper instead of recursing.

There were two fairly major complications.

Firstly, IRLinker::linkAppendingVarProto incorporates an on-the-fly IR
ugprade that I had to split apart.  Long-term, this upgrade should be
done in the bitcode reader (and we should only accept the "new" form),
but for now I've just made it work and added a FIXME.  The hold-op is
that we need to deprecate C API that relies on this.

Secondly, IRLinker has special logic to correctly implement aliases with
comdats, and uses two ValueToValueMapTy instances and two
ValueMaterializers.  I supported this by allowing clients to register an
alternate mapping context, whose MCID can be passed in when scheduling
new work.

While out of scope for this commit, it should now be straightforward to
remove recursion from Mapper::mapValue.

llvm-svn: 266503
2016-04-16 01:29:08 +00:00
Duncan P. N. Exon Smith 8a15dab391 Linker: Remove an unnecessary local variable in for loop, NFC
Reduces changes in a follow-up commit.

llvm-svn: 266493
2016-04-15 23:32:44 +00:00
Adrian Prantl 75819aedf6 [PR27284] Reverse the ownership between DICompileUnit and DISubprogram.
Currently each Function points to a DISubprogram and DISubprogram has a
scope field. For member functions the scope is a DICompositeType. DIScopes
point to the DICompileUnit to facilitate type uniquing.

Distinct DISubprograms (with isDefinition: true) are not part of the type
hierarchy and cannot be uniqued. This change removes the subprograms
list from DICompileUnit and instead adds a pointer to the owning compile
unit to distinct DISubprograms. This would make it easy for ThinLTO to
strip unneeded DISubprograms and their transitively referenced debug info.

Motivation
----------

Materializing DISubprograms is currently the most expensive operation when
doing a ThinLTO build of clang.

We want the DISubprogram to be stored in a separate Bitcode block (or the
same block as the function body) so we can avoid having to expensively
deserialize all DISubprograms together with the global metadata. If a
function has been inlined into another subprogram we need to store a
reference the block containing the inlined subprogram.

Attached to https://llvm.org/bugs/show_bug.cgi?id=27284 is a python script
that updates LLVM IR testcases to the new format.

http://reviews.llvm.org/D19034
<rdar://problem/25256815>

llvm-svn: 266446
2016-04-15 15:57:41 +00:00
Duncan P. N. Exon Smith bb2c3e199e ValueMapper: Extract llvm::RemapFunction from IRMover.cpp, NFC
Strip out the remapping parts of IRLinker::linkFunctionBody and put them
in ValueMapper.cpp under the name Mapper::remapFunction (with a
top-level entry-point llvm::RemapFunction).

This is a nice cleanup on its own since it puts the remapping code
together and shares a single Mapper context for the entire
IRLinker::linkFunctionBody Call.  Besides that, this will make it easier
to break the co-recursion between IRMover.cpp and ValueMapper.cpp in
follow ups.

llvm-svn: 265835
2016-04-08 19:26:32 +00:00
Duncan P. N. Exon Smith fe6583a0a6 Linker: Always pass RF_IgnoreMissingLocals; NFC
This is a cleanup after clarifying the meaning of RF_IgnoreMissingLocals
in r265628 and truly limiting it to locals in r265768.

This should have no functionality change, since the only context that
the flag has an effect is when we could hit function-local Value and
Metadata, and we were already passing it in those contexts.

llvm-svn: 265831
2016-04-08 19:01:38 +00:00
Duncan P. N. Exon Smith da68cbc4ad IR: RF_IgnoreMissingValues => RF_IgnoreMissingLocals, NFC
Clarify what this RemapFlag actually means.

  - Change the flag name to match its intended behaviour.
  - Clearly document that it's not supposed to affect globals.
  - Add a host of FIXMEs to indicate how to fix the behaviour to match
    the intent of the flag.

RF_IgnoreMissingLocals should only affect the behaviour of
RemapInstruction for function-local operands; namely, for operands of
type Argument, Instruction, and BasicBlock.  Currently, it is *only*
passed into RemapInstruction calls (and the transitive MapValue calls
that it makes).

When I split Metadata from Value I didn't understand the flag, and I
used it in a bunch of places for "global" metadata.

This commit doesn't have any functionality change, but prepares to
cleanup MapMetadata and MapValue.

llvm-svn: 265628
2016-04-07 00:26:43 +00:00
Duncan P. N. Exon Smith bdfc984679 IRMover: Steal arguments when moving functions, NFC
Instead of copying arguments from the source function to the
destination, steal them.  This has a few advantages.

  - The ValueMap doesn't need to be seeded with (or cleared of)
    Arguments.

  - Often the destination function won't have created any arguments yet,
    so this avoids malloc traffic.

  - Argument names don't need to be copied.

Because argument lists are lazy, this required a new
Function::stealArgumentListFrom helper.

llvm-svn: 265519
2016-04-06 06:38:15 +00:00
Duncan P. N. Exon Smith 6d72d166dc Linker: Split mapUnneededSubprograms into two; almost NFC
Split the loop through compile units in mapUnneededSubprograms in two.
First, visit imported entities to ensure that we've visited all need
subprograms.  Second, visit subprograms, and drop the ones we don't
need.

Hypothetically this protects against a subprogram from one compile unit
being referenced from an imported entity in a different compile unit.  I
don't think that's valid IR (a debug info expert could confirm), but I
think the refactor makes the code more clear.

llvm-svn: 265233
2016-04-02 17:54:01 +00:00
Duncan P. N. Exon Smith 751114b39d Remove redundant assertion after cast, NFC
llvm-svn: 265232
2016-04-02 17:41:52 +00:00
Duncan P. N. Exon Smith 0d60a9887f Linker: Avoid unnecessary work when moving named metadata
IRLinker::mapUnneededSubprograms has to be sure that any "needed"
subprograms get linked in.  Rather than traversing through imported
entities using llvm::getSubprogram, call MapMetadata.  The latter
memoizes the result in the ValueMap (sharing work with
IRLinker::linkNamedMDNodes proper), and makes the local SmallPtrSet
redundant.

llvm-svn: 265231
2016-04-02 17:39:31 +00:00
Duncan P. N. Exon Smith 4b520e5ef6 Linker: Remove IRMover::isMetadataUnneeded indirection; almost NFC
Instead of checking live during MapMetadata whether a subprogram is
needed, seed the ValueMap with `nullptr` up-front.

There is a small hypothetical functionality change.  Previously, calling
MapMetadataOp on a node whose "scope:" chain led to an unneeded
subprogram would return nullptr.  However, if that were ever called,
then the subprogram would be needed; a situation that the IRMover is
supposed to avoid a priori!

Besides cleaning up the code a little, this restores a nice property:
MapMetadataOp returns the same as MapMetadata.

llvm-svn: 265229
2016-04-02 17:12:00 +00:00
Duncan P. N. Exon Smith da4a56d1ab ValueMapper: Add support for seeding metadata with nullptr
Support seeding a ValueMap with nullptr for Metadata entries, a
situation I didn't consider in the Metadata/Value split.

I added a ValueMapper::getMappedMD accessor that returns an
Optional<Metadata*> with the mapped (possibly null) metadata.  IRMover
needs to use this to avoid modifying the map when it's checking for
unneeded subprograms.  I updated a call from bugpoint since I find the
new code clearer.

llvm-svn: 265228
2016-04-02 17:04:38 +00:00
Teresa Johnson b703c77b03 [ThinLTO] Remove post-pass metadata linking support
Since we have moved to a model where functions are imported in bulk from
each source module after making summary-based importing decisions, there
is no longer a need to link metadata as a postpass, and all users have
been removed.

This essentially reverts r255909 and follow-on fixes.

llvm-svn: 264763
2016-03-29 18:24:19 +00:00
Mehdi Amini 33661070c5 Minor cleanup and documentation to IRMover (NFC)
From: Mehdi Amini <mehdi.amini@apple.com>
llvm-svn: 263304
2016-03-11 22:19:06 +00:00
Teresa Johnson 0556e2217d Materialize metadata in IRLinker before value mapping
Summary:
Unless we plan to do later postpass metadata linking (ThinLTO special mode),
always invoke metadata materialization at the start of IRLinker::run().
This avoids the need for clients who use lazy metadata loading to
explicitly invoke materializeMetadata before the IRMover, which in
turn invokes IRLinker::run and needs materialized metadata for mapping.

Came up in the context of an LLD issue (D17982).

Reviewers: rafael

Subscribers: silvas, llvm-commits

Differential Revision: http://reviews.llvm.org/D17992

llvm-svn: 263143
2016-03-10 18:47:03 +00:00
Rafael Espindola 40358fb86f Pass a std::unique_ptr to IRMover::move.
It was already the one "destroying" the source module, now the API
reflects that.

llvm-svn: 260989
2016-02-16 18:50:12 +00:00
Teresa Johnson beb43ba7e5 Improve efficiency of handling unmapped subprogram metadata
The stripNullSubprograms function is very inefficient because
it walks all subprograms in all compile units in the dest module
any time a new module is linked in. For LTO in particular this will
get increasingly expensive as more modules are linked.

This patch improves the efficiency in several ways. The first is that
no scanning is necessary when there were no unneeded subprograms
identified in the first place. The second is that only the newly-linked
module's compile unit metadata should be examined.

Fixes PR26346.

llvm-svn: 259049
2016-01-28 15:08:09 +00:00
Teresa Johnson 71d12d2a4e [ThinLTO] Find all needed metadata when linking metadata as postpass
For metadata postpass linking, after importing all functions, we need
to recursively walk through any nodes reached via imported functions to
locate needed subprogram metadata. Some might only be reached indirectly
via the variable list for an inlined function.

llvm-svn: 258728
2016-01-25 22:04:56 +00:00
Teresa Johnson f07db00a65 [ThinLTO] Handle DISubprogram reached indirectly from DIImportedEntity
Extend fix for PR26037 to identify DISubprogram reached from a
DIImportedEntity via a DILexicalBlock.

llvm-svn: 258722
2016-01-25 21:29:55 +00:00
Teresa Johnson f5aa64f25f Use early return to simplify code (NFC)
Follow on to r258405.

llvm-svn: 258407
2016-01-21 17:16:53 +00:00
Teresa Johnson 6f508afce1 [ThinLTO] Avoid unnecesary hash lookups during metadata linking (NFC)
Replace sequences of count() followed by operator[] with either
find() or insert(), depending on the context.

llvm-svn: 258405
2016-01-21 16:46:40 +00:00
Rafael Espindola 394524d940 Remove redundant argument.
It is already a member variable.

llvm-svn: 258369
2016-01-21 00:00:53 +00:00