[PATCH] ValueMapper: Eliminate cross-file co-recursion, NFC

[Rafael Espíndola via llvm-commits]

On 12 April 2016 at 19:28, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 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.  Frankly I think 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 issue is not bitcode, is support for old clients and the C api.
Now that we have proper deprecation procedures we should probably do
it. In this case we are not deprecating a symbol (after all it is just
creating a GlobalVariable), we are deprecating what the expected
content should be.


> Secondly, IRLinker has special logic to correctly implement aliases with
> comdats, and uses two ValueToValueMapTy instances and two
> ValueMaterializers.  This wasn't hard to support by making ValueMapper
> support arbitrary MappingContexts when scheduling work.

Thanks for taking care of this odd case.

> While out of scope for this commit, it should be straightforward to
> remove recursion from Mapper::mapValue once this is in place.

Out of curiosity, is this making the algorithm more efficient,
avoiding overflowing the stack or is just a cleanup?

Cheers,
Rafael