Revert "ValueMapper: Make LocalAsMetadata match function-local Values"

This reverts commit r265631, since it caused bot failures:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3256
ht

Revert "ValueMapper: Make LocalAsMetadata match function-local Values"

This reverts commit r265631, since it caused bot failures:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3256
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-42vma/builds/7272

Looks like something is depending on the old behaviour. I'll try to
track it down and recommit.

llvm-svn: 265637

show more ...

ValueMapper: Allow RF_IgnoreMissingLocals and RF_NullMapMissingGlobalValues

Remove the assertion that disallowed the combination, since
RF_IgnoreMissingLocals should have no effect on globals. As i

ValueMapper: Allow RF_IgnoreMissingLocals and RF_NullMapMissingGlobalValues

Remove the assertion that disallowed the combination, since
RF_IgnoreMissingLocals should have no effect on globals. As it happens,
RF_NullMapMissingGlobalValues asserted in MapValue(Constant*,...), so I
also changed a cast to a cast_or_null to get my test passing.

llvm-svn: 265633

show more ...

ValueMapper: Make LocalAsMetadata match function-local Values

Start treating LocalAsMetadata similarly to function-local members of
the Value hierarchy in MapValue and MapMetadata.

- Don't memoiz

ValueMapper: Make LocalAsMetadata match function-local Values

Start treating LocalAsMetadata similarly to function-local members of
the Value hierarchy in MapValue and MapMetadata.

- Don't memoize them.
- Return nullptr if they are missing.

This also cleans up ConstantAsMetadata to stop listening to the
RF_IgnoreMissingLocals flag.

llvm-svn: 265631

show more ...

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 su

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

show more ...

ValueMapper: Fix delayed blockaddress handling after r265273

r265273 added Mapper::mapBlockAddress, which delays mapping a
blockaddress value until the function has a body. The condition was
backwa

ValueMapper: Fix delayed blockaddress handling after r265273

r265273 added Mapper::mapBlockAddress, which delays mapping a
blockaddress value until the function has a body. The condition was
backwards, and should be checking Function::empty instead of
GlobalValue::isDeclaration.

llvm-svn: 265508

show more ...

Try harder to appease MSVC after r265456

r265465 wasn't good enough. I need to spell out all the moves.

llvm-svn: 265470

Try to appease MSVC after r265456

I can't remember if adding `= default` will make MSVC happy, or if I
have to spell this out. Let's try the cleaner version first.

llvm-svn: 265465

ValueMapper: Rewrite Mapper::mapMetadata without recursion

This commit completely rewrites Mapper::mapMetadata (the implementation
of llvm::MapMetadata) using an iterative algorithm. The guts of th

ValueMapper: Rewrite Mapper::mapMetadata without recursion

This commit completely rewrites Mapper::mapMetadata (the implementation
of llvm::MapMetadata) using an iterative algorithm. The guts of the new
algorithm are in MDNodeMapper::map, the entry function in a new class.

Previously, Mapper::mapMetadata performed a recursive exploration of the
graph with eager "just in case there's a reason" malloc traffic.

The new algorithm has these benefits:

- New nodes and temporaries are not created eagerly.
- Uniquing cycles are not duplicated (see new unit test).
- No recursion.

Given a node to map, it does this:

1. Use a worklist to perform a post-order traversal of the transitively
referenced unmapped nodes.

2. Track which nodes will change operands, and which will have new
addresses in the mapped scheme. Propagate the changes through the
POT until fixed point, to pick up uniquing cycles that need to
change.

3. Map all the distinct nodes without touching their operands. If
RF_MoveDistinctMetadata, they get mapped to themselves; otherwise,
they get mapped to clones.

4. Map the uniqued nodes (bottom-up), lazily creating temporaries for
forward references as needed.

5. Remap the operands of the distinct nodes.

Mehdi helped me out by profiling this with -flto=thin. On his workload
(importing/etc. for opt.cpp), MapMetadata sped up by 15%, contributed
about 50% less to persistent memory, and made about 100x fewer calls to
malloc. The speedup is less than I'd hoped. The profile mainly blames
DenseMap lookups; perhaps there's a way to reduce them (e.g., by
disallowing remapping of MDString).

It would be nice to break the strange remaining recursion on the Value
side: MapValue => materializeInitFor => RemapInstruction => MapValue. I
think we could do this by having materializeInitFor return a worklist of
things to be remapped.

llvm-svn: 265456

show more ...

ValueMapper: Remove old FIXMEs; almost NFC

Remove a few old FIXMEs from the original commit of the Metadata/Value
split in r223802. These are commented out assertions to the effect that
calls betwe

ValueMapper: Remove old FIXMEs; almost NFC

Remove a few old FIXMEs from the original commit of the Metadata/Value
split in r223802. These are commented out assertions to the effect that
calls between mapValue and mapMetadata never return nullptr.

(The only behaviour change is that Mapper::mapSimpleMetadata memoizes
the nullptr return.)

When I originally rewrote the mapping code, I thought we could be
stricter in the new metadata hierarchy and never return nullptr when
RF_NullMapMissingGlobalValues was off. It's still not entirely clear to
me why these assertions failed (a few months ago, I had a theory that I
forgot to write down, but that's helping no one).

Understood or not, I no longer see how these commented-out assertions
would be useful. I'm relegating them to the annals of source control
before making significant changes to ValueMapper.cpp.

llvm-svn: 265282

show more ...

ValueMapper: Disallow metadata mapping recursion through mapValue

This adds an assertion to maintain the property from r265273. When
Mapper::mapSimpleMetadata calls Mapper::mapValue, it should not

ValueMapper: Disallow metadata mapping recursion through mapValue

This adds an assertion to maintain the property from r265273. When
Mapper::mapSimpleMetadata calls Mapper::mapValue, it should not find its
way back to mapMetadataImpl. This guarantees that mapSimpleMetadata is
not involved in any recursion.

Since Mapper::mapValue calls out to arbitrary materializers, we need to
save a bit on the ValueMap to make this assertion effective.

There should be no functionality change here. This co-recursion should
already have been impossible.

llvm-svn: 265276

show more ...

Work around MSVC failure from r265273

http://lab.llvm.org:8011/builders/sanitizer-windows/builds/19726

llvm-svn: 265275

ValueMapper: Avoid recursion in mapSimplifiedMetadata, NFC

The main change is to delay materializing GlobalValue initializers from
Mapper::mapValue until Mapper::~Mapper. This effectively removes a

ValueMapper: Avoid recursion in mapSimplifiedMetadata, NFC

The main change is to delay materializing GlobalValue initializers from
Mapper::mapValue until Mapper::~Mapper. This effectively removes all
recursion from mapSimplifiedMetadata, as promised in r265270.
mapSimplifiedMetadata calls mapValue for ConstantAsMetadata nodes to
find the mapped constant, and now it shouldn't be possible for mapValue
to indirectly re-invoke mapMetadata. I'll add an assertion to that
effect in a follow-up (separated so that the assertion can easily be
reverted independently, if it comes to that).

This a step toward a broader goal: converting Mapper::mapMetadataImpl
from a recursive to an iterative algorithm.

When a BlockAddress points at a BasicBlock inside an unmaterialized
function body, we need to delay it until the function body is
materialized in Mapper::~Mapper. This commit creates a temporary
BasicBlock and returns a new BlockAddress, then RAUWs the BasicBlock
once it is known. This situation should be extremely rare since a
BlockAddress is usually used from within the function it's referencing
(and BlockAddress itself is rare).

There should be no observable functionality change.

llvm-svn: 265273

show more ...

ValueMapper: Split out mapSimpleMetadata, NFC

Split out a helper for mapping metadata without operands. This is any
metadata that is not an MDNode, and any MDNode where the answer is known
without

ValueMapper: Split out mapSimpleMetadata, NFC

Split out a helper for mapping metadata without operands. This is any
metadata that is not an MDNode, and any MDNode where the answer is known
without looking at operands.

Through some weird twists, this function is co-recursive:

mapSimpleMetadata
=> MapValue
=> materializeInitFor
=> linkFunctionBody
=> RemapInstructions
=> MapMetadata
=> mapSimpleMetadata

I plan to break the recursion in a follow-up.

llvm-svn: 265270

show more ...

ValueMapper: Introduce Mapper helper class, NFC

Remove a bunch of boilerplate from ValueMapper.cpp by using a new
file-local class called Mapper.

llvm-svn: 265268

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

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

show more ...

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 ValueMa

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

show more ...

[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

[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

show more ...

Split resolveCycles(bool AllowTemps) into two interfaces and document

Address review feedback from r255909.

Move body of resolveCycles(bool AllowTemps) to
resolveRecursivelyImpl(bool AllowTemps). R

Split resolveCycles(bool AllowTemps) into two interfaces and document

Address review feedback from r255909.

Move body of resolveCycles(bool AllowTemps) to
resolveRecursivelyImpl(bool AllowTemps). Revert resolveCycles back
to asserting on temps, and add new resolveNonTemporaries interface
to invoke the new implementation with AllowTemps=true. Document
the differences between these interfaces, specifically the effect
on RAUW support and uniquing. Call appropriate interface from
ValueMapper.

llvm-svn: 257389

show more ...

[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
sav

[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

show more ...

[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

[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

show more ...

[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,
suturin

[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

show more ...

Bring r254336 back:

The difference is that now we don't error on out-of-comdat access to
internal global values. We copy them instead. This seems to match the
expectation of COFF linkers (see pr2568

Bring r254336 back:

The difference is that now we don't error on out-of-comdat access to
internal global values. We copy them instead. This seems to match the
expectation of COFF linkers (see pr25686).

Original message:

Start deciding earlier what to link.

A traditional linker is roughly split in symbol resolution and
"copying
stuff".

The two tasks are badly mixed in lib/Linker.

This starts splitting them apart.

With this patch there are no direct call to linkGlobalValueBody or
linkGlobalValueProto. Everything is linked via WapValue.

This also includes a few fixes:
* A GV goes undefined if the comdat is dropped (comdat11.ll).
* We error if an internal GV goes undefined (comdat13.ll).
* We don't link an unused comdat.

The first two match the behavior of an ELF linker. The second one is
equivalent to running globaldce on the input.

llvm-svn: 254418

show more ...

This reverts commit r254336 and r254344.

They broke a bot and I am debugging why.

llvm-svn: 254347

Start deciding earlier what to link.

A traditional linker is roughly split in symbol resolution and "copying
stuff".

The two tasks are badly mixed in lib/Linker.

This starts splitting them apart.

Start deciding earlier what to link.

A traditional linker is roughly split in symbol resolution and "copying
stuff".

The two tasks are badly mixed in lib/Linker.

This starts splitting them apart.

With this patch there are no direct call to linkGlobalValueBody or
linkGlobalValueProto. Everything is linked via WapValue.

This also includes a few fixes:
* A GV goes undefined if the comdat is dropped (comdat11.ll).
* We error if an internal GV goes undefined (comdat13.ll).
* We don't link an unused comdat.

The first two match the behavior of an ELF linker. The second one is
equivalent to running globaldce on the input.

llvm-svn: 254336

show more ...

Simplify the linking of recursive data.

Now the ValueMapper has two callbacks. The first one maps the
declaration. The ValueMapper records the mapping and then materializes
the body/initializer.

ll

Simplify the linking of recursive data.

Now the ValueMapper has two callbacks. The first one maps the
declaration. The ValueMapper records the mapping and then materializes
the body/initializer.

llvm-svn: 254209

show more ...