Commit Graph

92 Commits

Author SHA1 Message Date
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
Rafael Espindola 55a7ae5cc7 Simplify the logic. NFC.
Found while reviewing the change for PR26152.

llvm-svn: 258362
2016-01-20 22:38:23 +00:00
Evgeniy Stepanov 9fb70f53ce Fix PR26152.
Fix the condition for when the new global takes over the name of
the existing one to be the negation of the condition for the new
global to get internal linkage.

llvm-svn: 258355
2016-01-20 22:05:50 +00:00
Manuel Jacob 5f6eaac611 GlobalValue: use getValueType() instead of getType()->getPointerElementType().
Reviewers: mjacob

Subscribers: jholewinski, arsenm, dsanders, dblaikie

Patch by Eduard Burtescu.

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

llvm-svn: 257999
2016-01-16 20:30:46 +00:00
Teresa Johnson 5fe40050bd [IRMover] Don't copy personality, etc unless creating def
Function::copyAttributesFrom will copy the personality function, prefix
data and prolog data from the source function to the new function, and
is invoked when the IRMover copies the function prototype. This puts a
reference to a constant in the source module on a function in the dest
module, which causes an error when deleting the source module after
importing, since the personality function in the source module still has
uses (this would presumably also be an issue for the prologue and prefix
data). Remove the copies added to the dest copy when creating the new
prototype, as they are mapped properly when/if we link the function body.

llvm-svn: 257420
2016-01-12 00:24:24 +00:00
Ahmed Bougacha a7324a2823 [Linker] Also treat a DIImportedEntity scope DISubprogram as needed.
Follow-up to r257000: DIImportedEntity can reach a DISubprogram via
its entity, but also via its scope. Handle the latter case as well.

PR26037.

llvm-svn: 257019
2016-01-07 03:14:59 +00:00
Teresa Johnson b951558294 Always treat DISubprogram reached by DIImportedEntity as needed.
It is illegal to have a null entity in a DIImportedEntity, so
we must link in a DISubprogram metadata node referenced by one,
even if the associated function is not linked in or inlined anywhere.

Fixes PR26037.

llvm-svn: 257000
2016-01-07 00:06:27 +00:00
Teresa Johnson 96f7f81aa3 [ThinLTO] Rename variables used in metadata linking (NFC)
As suggested in review for r255909, rename MDMaterialized to AllowTemps,
and identify the name of the boolean flag being set in calls to
saveMetadataList.

llvm-svn: 256653
2015-12-30 21:13:55 +00:00
Teresa Johnson cc428573a2 Ensure MDNode used as key in metadata linking map cannot be RAUWed
As suggested in review for r255909, add a way to ensure that temporary
MD used as keys in the MetadataToID map during ThinLTO importing are not
RAUWed.

Add support for marking an MDNode as not replaceable. Clear the new
CanReplace flag when adding a temporary MD node to the MetadataToID map
and clear it when destroying the map.

llvm-svn: 256648
2015-12-30 19:32:24 +00:00
Teresa Johnson 61b406ec75 Rename MDValue* to Metadata* (NFC)
Renamed MDValue* to Metadata*, and MDValueToValIDMap to MetadataToIDs,
as per review for r255909.

llvm-svn: 256593
2015-12-29 23:00:22 +00:00
Teresa Johnson d213aa469e Handle empty Subprogram list when linking metadata.
Use an iterator that handles an empty subprogram list.

Fixes PR25915.

llvm-svn: 256224
2015-12-22 01:17:19 +00:00
Rafael Espindola 18c63b0f18 Drop support for dematerializing.
It was only used on lib/Linker and the use was "dead" since it was used on a
function the IRMover had just moved.

llvm-svn: 256019
2015-12-18 19:57:26 +00:00
Teresa Johnson bef543635a Rename variables to reflect linker split (NFC)
Renamed variables to be more reflective of whether they are
an instance of Linker, IRLinker or ModuleLinker. Also fix a stale
comment.

llvm-svn: 256011
2015-12-18 19:28:59 +00:00
Teresa Johnson 0e7c82cb69 [ThinLTO/LTO] Don't link in unneeded metadata
Summary:
Third patch split out from http://reviews.llvm.org/D14752.

Only map in needed DISubroutine metadata (imported or otherwise linked
in functions and other DISubroutine referenced by inlined instructions).
This is supported for ThinLTO, LTO and llvm-link --only-needed, with
associated tests for each one.

Depends on D14838.

Reviewers: dexonsmith, joker.eph

Subscribers: davidxl, llvm-commits, joker.eph

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

llvm-svn: 256003
2015-12-18 17:51:37 +00:00
Teresa Johnson e5a6191732 [ThinLTO] Metadata linking for imported functions
Summary:
Second patch split out from http://reviews.llvm.org/D14752.

Maps metadata as a post-pass from each module when importing complete,
suturing up final metadata to the temporary metadata left on the
imported instructions.

This entails saving the mapping from bitcode value id to temporary
metadata in the importing pass, and from bitcode value id to final
metadata during the metadata linking postpass.

Depends on D14825.

Reviewers: dexonsmith, joker.eph

Subscribers: davidxl, llvm-commits, joker.eph

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

llvm-svn: 255909
2015-12-17 17:14:09 +00:00
Rafael Espindola 9d2bfc4874 Use diagnostic handler in the LLVMContext
This patch converts code that has access to a LLVMContext to not take a
diagnostic handler.

This has a few advantages

* It is easier to use a consistent diagnostic handler in a single program.
* Less clutter since we are not passing a handler around.

It does make it a bit awkward to implement some C APIs that return a
diagnostic string. I will propose new versions of these APIs and
deprecate the current ones.

llvm-svn: 255571
2015-12-14 23:17:03 +00:00
Rafael Espindola a8547d35e9 Fix another case where the linkage was not set.
llvm-svn: 255272
2015-12-10 18:44:26 +00:00
Rafael Espindola caabe22832 Slit lib/Linker in two.
A linker normally has two stages: symbol resolution and "moving stuff".

In lib/Linker there is the complication of lazy linking some globals,
but it was still far more mixed than it needed to.

This splits the linker into a lower level IRMover and the linker proper.
The IRMover just takes a list of globals to move and a callback that
lets the user control what is lazy linked.

The main motivation is that now tools/gold (and soon lld) can use their
own symbol resolution to instruct IRMover what to do.

llvm-svn: 255254
2015-12-10 14:19:35 +00:00