[llvm] r265456 - ValueMapper: Rewrite Mapper::mapMetadata without recursion
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 5 14:07:28 PDT 2016
Looking.
> On 2016-Apr-05, at 14:05, Mike Aizatsky <aizatsky at google.com> wrote:
>
> It seems that this changes breaks sanitizer-windows bot:
>
> http://lab.llvm.org:8011/builders/sanitizer-windows/builds/19822/steps/run%20tests/logs/stdio
>
> [162/1466] Building CXX object lib\LTO\CMakeFiles\LLVMLTO.dir\LTOCodeGenerator.cpp.obj
> FAILED: C:\PROGRA~2\MICROS~1.0\VC\bin\cl.exe /nologo /TP /DWIN32 /D_WINDOWS /W3 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /Zc:inline /Oi /Zc:rvalueCast /MD /O2 /Ob2 -Ilib\Transforms\Utils -IC:\b\slave\sanitizer-windows\llvm\lib\Transforms\Utils -Iinclude -IC:\b\slave\sanitizer-windows\llvm\include -UNDEBUG /EHs-c- /GR- /showIncludes -DGTEST_HAS_RTTI=0 -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_DEBUG_POINTER_IMPL="" -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS /Folib\Transforms\Utils\CMakeFiles\LLVMTransformUtils.dir\ValueMapper.cpp.obj /Fdlib\Transforms\Utils\CMakeFiles\LLVMTransformUtils.dir\ /FS -c C:\b\slave\sanitizer-windows\llvm\lib\Transforms\Utils\ValueMapper.cpp
> C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\INCLUDE\xstring(792) : error C2280: 'std::unique_ptr<llvm::MDNode,llvm::TempMDNodeDeleter>::unique_ptr(const std::unique_ptr<llvm::MDNode,llvm::TempMDNodeDeleter> &)' : attempting to reference a deleted function
> C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\INCLUDE\memory(1486) : see declaration of 'std::unique_ptr<llvm::MDNode,llvm::TempMDNodeDeleter>::unique_ptr'
> This diagnostic occurred in the compiler generated function '`anonymous-namespace'::MDNodeMapper::Data::Data(const `anonymous-namespace'::MDNodeMapper::Data &)'
> ninja: build stopped: subcommand failed.
>
>
> On Tue, Apr 5, 2016 at 1:28 PM Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> Author: dexonsmith
> Date: Tue Apr 5 15:23:21 2016
> New Revision: 265456
>
> URL: http://llvm.org/viewvc/llvm-project?rev=265456&view=rev
> Log:
> 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.
>
> Modified:
> llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
> llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=265456&r1=265455&r2=265456&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Tue Apr 5 15:23:21 2016
> @@ -58,7 +58,10 @@ struct DelayedBasicBlock {
> TempBB(BasicBlock::Create(Old.getContext())) {}
> };
>
> +class MDNodeMapper;
> class Mapper {
> + friend class MDNodeMapper;
> +
> ValueToValueMapTy &VM;
> RemapFlags Flags;
> ValueMapTypeRemapper *TypeMapper;
> @@ -66,7 +69,6 @@ class Mapper {
>
> SmallVector<DelayedGlobalValueInit, 8> DelayedInits;
> SmallVector<DelayedBasicBlock, 1> DelayedBBs;
> - SmallVector<MDNode *, 8> DistinctWorklist;
>
> public:
> Mapper(ValueToValueMapTy &VM, RemapFlags Flags,
> @@ -87,39 +89,146 @@ public:
> private:
> Value *mapBlockAddress(const BlockAddress &BA);
>
> - /// Map metadata helper.
> - ///
> - /// Co-recursively finds the mapping for MD. If this returns an MDNode, it's
> - /// possible that MDNode::isResolved() will return false.
> - Metadata *mapMetadataImpl(const Metadata *MD);
> - Metadata *mapMetadataOp(Metadata *Op);
> -
> /// Map metadata that doesn't require visiting operands.
> Optional<Metadata *> mapSimpleMetadata(const Metadata *MD);
>
> - /// Remap the operands of an MDNode.
> + Metadata *mapToMetadata(const Metadata *Key, Metadata *Val);
> + Metadata *mapToSelf(const Metadata *MD);
> +};
> +
> +class MDNodeMapper {
> + Mapper &M;
> +
> + struct Data {
> + bool HasChangedOps = false;
> + bool HasChangedAddress = false;
> + unsigned ID = ~0u;
> + TempMDNode Placeholder;
> + };
> +
> + SmallDenseMap<const Metadata *, Data, 32> Info;
> + SmallVector<std::pair<MDNode *, bool>, 16> Worklist;
> + SmallVector<MDNode *, 16> POT;
> +
> +public:
> + MDNodeMapper(Mapper &M) : M(M) {}
> +
> + /// Map a metadata node (and its transitive operands).
> ///
> - /// If \c Node is temporary, uniquing cycles are ignored. If \c Node is
> - /// distinct, uniquing cycles are resolved as they're found.
> + /// This is the only entry point into MDNodeMapper. It works as follows:
> ///
> - /// \pre \c Node.isDistinct() or \c Node.isTemporary().
> - bool remapOperands(MDNode &Node);
> + /// 1. \a createPOT(): use a worklist to perform a post-order traversal of
> + /// the transitively referenced unmapped nodes.
> + ///
> + /// 2. \a propagateChangedOperands(): 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. \a mapDistinctNodes(): map all the distinct nodes without touching
> + /// their operands. If RF_MoveDistinctMetadata, they get mapped to
> + /// themselves; otherwise, they get mapped to clones.
> + ///
> + /// 4. \a mapUniquedNodes(): map the uniqued nodes (bottom-up), lazily
> + /// creating temporaries for forward references as needed.
> + ///
> + /// 5. \a remapDistinctOperands(): remap the operands of the distinct nodes.
> + Metadata *map(const MDNode &FirstN);
> +
> +private:
> + /// Return \c true as long as there's work to do.
> + bool hasWork() const { return !Worklist.empty(); }
>
> - /// Map a distinct MDNode.
> + /// Get the current node in the worklist.
> + MDNode &getCurrentNode() const { return *Worklist.back().first; }
> +
> + /// Push a node onto the worklist.
> + ///
> + /// Adds \c N to \a Worklist and \a Info, unless it's already inserted. If
> + /// \c N.isDistinct(), \a Data::HasChangedAddress will be set based on \a
> + /// RF_MoveDistinctMDs.
> + ///
> + /// Returns the data for the node.
> ///
> - /// Whether distinct nodes change is independent of their operands. If \a
> - /// RF_MoveDistinctMDs, then they are reused, and their operands remapped in
> - /// place; effectively, they're moved from one graph to another. Otherwise,
> - /// they're cloned/duplicated, and the new copy's operands are remapped.
> - Metadata *mapDistinctNode(const MDNode *Node);
> + /// \post Data::HasChangedAddress iff !RF_MoveDistinctMDs && N.isDistinct().
> + /// \post Worklist.back().first == &N.
> + /// \post Worklist.back().second == false.
> + Data &push(const MDNode &N);
>
> - /// Map a uniqued MDNode.
> + /// Map a node operand, and return true if it changes.
> ///
> - /// Uniqued nodes may not need to be recreated (they may map to themselves).
> - Metadata *mapUniquedNode(const MDNode *Node);
> + /// \post getMappedOp(Op) does not return None.
> + bool mapOperand(const Metadata *Op);
>
> - Metadata *mapToMetadata(const Metadata *Key, Metadata *Val);
> - Metadata *mapToSelf(const Metadata *MD);
> + /// Get a previously mapped node.
> + Optional<Metadata *> getMappedOp(const Metadata *Op) const;
> +
> + /// Try to pop a node off the worklist and store it in POT.
> + ///
> + /// Returns \c true if it popped; \c false if its operands need to be
> + /// visited.
> + ///
> + /// \post If Worklist.back().second == false: Worklist.back().second == true.
> + /// \post Else: Worklist.back() has been popped off and added to \a POT.
> + bool tryToPop();
> +
> + /// Get a forward reference to a node to use as an operand.
> + ///
> + /// Returns \c Op if it's not changing; otherwise, lazily creates a temporary
> + /// node and returns it.
> + Metadata &getFwdReference(const Data &D, MDNode &Op);
> +
> + /// Create a post-order traversal from the given node.
> + ///
> + /// This traverses the metadata graph deeply enough to map \c FirstN. It
> + /// uses \a mapOperand() (indirectly, \a Mapper::mapSimplifiedNode()), so any
> + /// metadata that has already been mapped will not be part of the POT.
> + ///
> + /// \post \a POT is a post-order traversal ending with \c FirstN.
> + bool createPOT(const MDNode &FirstN);
> +
> + /// Propagate changed operands through post-order traversal.
> + ///
> + /// Until fixed point, iteratively update:
> + ///
> + /// - \a Data::HasChangedOps based on \a Data::HasChangedAddress of operands;
> + /// - \a Data::HasChangedAddress based on Data::HasChangedOps.
> + ///
> + /// This algorithm never changes \a Data::HasChangedAddress for distinct
> + /// nodes.
> + ///
> + /// \post \a POT is a post-order traversal ending with \c FirstN.
> + void propagateChangedOperands();
> +
> + /// Map all distinct nodes in POT.
> + ///
> + /// \post \a getMappedOp() returns the correct node for every distinct node.
> + void mapDistinctNodes();
> +
> + /// Map all uniqued nodes in POT with the correct operands.
> + ///
> + /// \pre Distinct nodes are mapped (\a mapDistinctNodes() has been called).
> + /// \post \a getMappedOp() returns the correct node for every node.
> + /// \post \a MDNode::operands() is correct for every uniqued node.
> + /// \post \a MDNode::isResolved() returns true for every node.
> + void mapUniquedNodes();
> +
> + /// Re-map the operands for distinct nodes in POT.
> + ///
> + /// \pre Distinct nodes are mapped (\a mapDistinctNodes() has been called).
> + /// \pre Uniqued nodes are mapped (\a mapUniquedNodes() has been called).
> + /// \post \a MDNode::operands() is correct for every distinct node.
> + void remapDistinctOperands();
> +
> + /// Remap a node's operands.
> + ///
> + /// Iterate through operands and update them in place using \a getMappedOp()
> + /// and \a getFwdReference().
> + ///
> + /// \pre N.isDistinct() or N.isTemporary().
> + /// \pre Distinct nodes are mapped (\a mapDistinctNodes() has been called).
> + /// \pre If \c N is distinct, all uniqued nodes are already mapped.
> + void remapOperands(const Data &D, MDNode &N);
> };
>
> } // end namespace
> @@ -280,78 +389,218 @@ Metadata *Mapper::mapToSelf(const Metada
> return mapToMetadata(MD, const_cast<Metadata *>(MD));
> }
>
> -Metadata *Mapper::mapMetadataOp(Metadata *Op) {
> +bool MDNodeMapper::mapOperand(const Metadata *Op) {
> + if (!Op)
> + return false;
> +
> + if (Optional<Metadata *> MappedOp = M.mapSimpleMetadata(Op)) {
> + assert(M.VM.getMappedMD(Op) && "Expected result to be memoized");
> + return *MappedOp != Op;
> + }
> +
> + return push(*cast<MDNode>(Op)).HasChangedAddress;
> +}
> +
> +Optional<Metadata *> MDNodeMapper::getMappedOp(const Metadata *Op) const {
> if (!Op)
> return nullptr;
>
> - if (Metadata *MappedOp = mapMetadataImpl(Op))
> - return MappedOp;
> - // Use identity map if MappedOp is null and we can ignore missing entries.
> - if (Flags & RF_IgnoreMissingEntries)
> + if (Optional<Metadata *> MappedOp = M.VM.getMappedMD(Op))
> + return *MappedOp;
> +
> + return None;
> +}
> +
> +Metadata &MDNodeMapper::getFwdReference(const Data &D, MDNode &Op) {
> + auto Where = Info.find(&Op);
> + assert(Where != Info.end() && "Expected a valid reference");
> +
> + auto &OpD = Where->second;
> + assert(OpD.ID > D.ID && "Expected a forward reference");
> +
> + if (!OpD.HasChangedAddress)
> return Op;
>
> - return nullptr;
> + // Lazily construct a temporary node.
> + if (!OpD.Placeholder)
> + OpD.Placeholder = Op.clone();
> +
> + return *OpD.Placeholder;
> +}
> +
> +void MDNodeMapper::remapOperands(const Data &D, MDNode &N) {
> + for (unsigned I = 0, E = N.getNumOperands(); I != E; ++I) {
> + Metadata *Old = N.getOperand(I);
> + Metadata *New;
> + if (Optional<Metadata *> MappedOp = getMappedOp(Old)){
> + New = *MappedOp;
> + } else {
> + assert(!N.isDistinct() &&
> + "Expected all nodes to be pre-mapped for distinct operands");
> + MDNode &OldN = *cast<MDNode>(Old);
> + assert(!OldN.isDistinct() && "Expected distinct nodes to be pre-mapped");
> + New = &getFwdReference(D, OldN);
> + }
> +
> + if (Old != New)
> + N.replaceOperandWith(I, New);
> + }
> }
>
> -/// Resolve uniquing cycles involving the given metadata.
> -static void resolveCycles(Metadata *MD) {
> - if (auto *N = dyn_cast_or_null<MDNode>(MD))
> - if (!N->isResolved())
> - N->resolveCycles();
> +MDNodeMapper::Data &MDNodeMapper::push(const MDNode &N) {
> + auto Insertion = Info.insert(std::make_pair(&N, Data()));
> + auto &D = Insertion.first->second;
> + if (!Insertion.second)
> + return D;
> +
> + // Add to the worklist; check for distinct nodes that are required to be
> + // copied.
> + Worklist.push_back(std::make_pair(&const_cast<MDNode &>(N), false));
> + D.HasChangedAddress = !(M.Flags & RF_MoveDistinctMDs) && N.isDistinct();
> + return D;
> +}
> +
> +bool MDNodeMapper::tryToPop() {
> + if (!Worklist.back().second) {
> + Worklist.back().second = true;
> + return false;
> + }
> +
> + MDNode *N = Worklist.pop_back_val().first;
> + Info[N].ID = POT.size();
> + POT.push_back(N);
> + return true;
> +}
> +
> +bool MDNodeMapper::createPOT(const MDNode &FirstN) {
> + bool AnyChanges = false;
> +
> + // Do a traversal of the unmapped subgraph, tracking whether operands change.
> + // In some cases, these changes will propagate naturally, but
> + // propagateChangedOperands() catches the general case.
> + AnyChanges |= push(FirstN).HasChangedAddress;
> + while (hasWork()) {
> + if (tryToPop())
> + continue;
> +
> + MDNode &N = getCurrentNode();
> + bool LocalChanges = false;
> + for (const Metadata *Op : N.operands())
> + LocalChanges |= mapOperand(Op);
> +
> + if (!LocalChanges)
> + continue;
> +
> + AnyChanges = true;
> + auto &D = Info[&N];
> + D.HasChangedOps = true;
> +
> + // Uniqued nodes change address when operands change.
> + if (!N.isDistinct())
> + D.HasChangedAddress = true;
> + }
> + return AnyChanges;
> +}
> +
> +void MDNodeMapper::propagateChangedOperands() {
> + bool AnyChangedAddresses;
> + do {
> + AnyChangedAddresses = false;
> + for (MDNode *N : POT) {
> + auto &NI = Info[N];
> + if (NI.HasChangedOps)
> + continue;
> +
> + if (!llvm::any_of(N->operands(), [&](const Metadata *Op) {
> + auto Where = Info.find(Op);
> + return Where != Info.end() && Where->second.HasChangedAddress;
> + }))
> + continue;
> +
> + NI.HasChangedOps = true;
> + if (!N->isDistinct()) {
> + NI.HasChangedAddress = true;
> + AnyChangedAddresses = true;
> + }
> + }
> + } while (AnyChangedAddresses);
> +}
> +
> +void MDNodeMapper::mapDistinctNodes() {
> + // Map all the distinct nodes in POT.
> + for (MDNode *N : POT) {
> + if (!N->isDistinct())
> + continue;
> +
> + if (M.Flags & RF_MoveDistinctMDs)
> + M.mapToSelf(N);
> + else
> + M.mapToMetadata(N, MDNode::replaceWithDistinct(N->clone()));
> + }
> }
>
> -bool Mapper::remapOperands(MDNode &Node) {
> - assert(!Node.isUniqued() && "Expected temporary or distinct node");
> - const bool IsDistinct = Node.isDistinct();
> -
> - bool AnyChanged = false;
> - for (unsigned I = 0, E = Node.getNumOperands(); I != E; ++I) {
> - Metadata *Old = Node.getOperand(I);
> - Metadata *New = mapMetadataOp(Old);
> - if (Old != New) {
> - AnyChanged = true;
> - Node.replaceOperandWith(I, New);
> -
> - // Resolve uniquing cycles underneath distinct nodes on the fly so they
> - // don't infect later operands.
> - if (IsDistinct)
> - resolveCycles(New);
> +void MDNodeMapper::mapUniquedNodes() {
> + // Construct uniqued nodes, building forward references as necessary.
> + for (auto *N : POT) {
> + if (N->isDistinct())
> + continue;
> +
> + auto &D = Info[N];
> + assert(D.HasChangedAddress == D.HasChangedOps &&
> + "Uniqued nodes should change address iff ops change");
> + if (!D.HasChangedAddress) {
> + M.mapToSelf(N);
> + continue;
> }
> +
> + TempMDNode ClonedN = D.Placeholder ? std::move(D.Placeholder) : N->clone();
> + remapOperands(D, *ClonedN);
> + M.mapToMetadata(N, MDNode::replaceWithUniqued(std::move(ClonedN)));
> }
>
> - return AnyChanged;
> + // Resolve cycles.
> + for (auto *N : POT)
> + if (!N->isResolved())
> + N->resolveCycles();
> }
>
> -Metadata *Mapper::mapDistinctNode(const MDNode *Node) {
> - assert(Node->isDistinct() && "Expected distinct node");
> +void MDNodeMapper::remapDistinctOperands() {
> + for (auto *N : POT) {
> + if (!N->isDistinct())
> + continue;
>
> - MDNode *NewMD;
> - if (Flags & RF_MoveDistinctMDs)
> - NewMD = const_cast<MDNode *>(Node);
> - else
> - NewMD = MDNode::replaceWithDistinct(Node->clone());
> + auto &D = Info[N];
> + if (!D.HasChangedOps)
> + continue;
>
> - // Remap operands later.
> - DistinctWorklist.push_back(NewMD);
> - return mapToMetadata(Node, NewMD);
> + assert(D.HasChangedAddress == !bool(M.Flags & RF_MoveDistinctMDs) &&
> + "Distinct nodes should change address iff they cannot be moved");
> + remapOperands(D, D.HasChangedAddress ? *cast<MDNode>(*getMappedOp(N)) : *N);
> + }
> }
>
> -Metadata *Mapper::mapUniquedNode(const MDNode *Node) {
> - assert(Node->isUniqued() && "Expected uniqued node");
> +Metadata *MDNodeMapper::map(const MDNode &FirstN) {
> + assert(!(M.Flags & RF_NoModuleLevelChanges) &&
> + "MDNodeMapper::map assumes module-level changes");
> + assert(POT.empty() && "MDNodeMapper::map is not re-entrant");
>
> - // Create a temporary node and map it upfront in case we have a uniquing
> - // cycle. If necessary, this mapping will get updated by RAUW logic before
> - // returning.
> - auto ClonedMD = Node->clone();
> - mapToMetadata(Node, ClonedMD.get());
> - if (!remapOperands(*ClonedMD)) {
> - // No operands changed, so use the original.
> - ClonedMD->replaceAllUsesWith(const_cast<MDNode *>(Node));
> - return const_cast<MDNode *>(Node);
> - }
> + // Require resolved nodes whenever metadata might be remapped.
> + assert(FirstN.isResolved() && "Unexpected unresolved node");
>
> - // Uniquify the cloned node.
> - return MDNode::replaceWithUniqued(std::move(ClonedMD));
> + // Return early if nothing at all changed.
> + if (!createPOT(FirstN)) {
> + for (const MDNode *N : POT)
> + M.mapToSelf(N);
> + return &const_cast<MDNode &>(FirstN);
> + }
> +
> + propagateChangedOperands();
> + mapDistinctNodes();
> + mapUniquedNodes();
> + remapDistinctOperands();
> +
> + // Return the original node, remapped.
> + return *getMappedOp(&FirstN);
> }
>
> Optional<Metadata *> Mapper::mapSimpleMetadata(const Metadata *MD) {
> @@ -389,21 +638,6 @@ Optional<Metadata *> Mapper::mapSimpleMe
> return None;
> }
>
> -Metadata *Mapper::mapMetadataImpl(const Metadata *MD) {
> - assert(VM.mayMapMetadata() && "Unexpected co-recursion through mapValue");
> - if (Optional<Metadata *> NewMD = mapSimpleMetadata(MD))
> - return *NewMD;
> -
> - // Require resolved nodes whenever metadata might be remapped.
> - auto *Node = cast<MDNode>(MD);
> - assert(Node->isResolved() && "Unexpected unresolved node");
> -
> - if (Node->isDistinct())
> - return mapDistinctNode(Node);
> -
> - return mapUniquedNode(Node);
> -}
> -
> Metadata *llvm::MapMetadata(const Metadata *MD, ValueToValueMapTy &VM,
> RemapFlags Flags, ValueMapTypeRemapper *TypeMapper,
> ValueMaterializer *Materializer) {
> @@ -411,25 +645,13 @@ Metadata *llvm::MapMetadata(const Metada
> }
>
> Metadata *Mapper::mapMetadata(const Metadata *MD) {
> - Metadata *NewMD = mapMetadataImpl(MD);
> -
> - // When there are no module-level changes, it's possible that the metadata
> - // graph has temporaries. Skip the logic to resolve cycles, since it's
> - // unnecessary (and invalid) in that case.
> - if (Flags & RF_NoModuleLevelChanges)
> - return NewMD;
> -
> - // Resolve cycles involving the entry metadata.
> - resolveCycles(NewMD);
> + if (Optional<Metadata *> NewMD = mapSimpleMetadata(MD))
> + return *NewMD;
>
> - return NewMD;
> + return MDNodeMapper(*this).map(*cast<MDNode>(MD));
> }
>
> Mapper::~Mapper() {
> - // Remap the operands of distinct MDNodes.
> - while (!DistinctWorklist.empty())
> - remapOperands(*DistinctWorklist.pop_back_val());
> -
> // Materialize global initializers.
> while (!DelayedInits.empty()) {
> auto Init = DelayedInits.pop_back_val();
> @@ -443,8 +665,7 @@ Mapper::~Mapper() {
> DBB.TempBB->replaceAllUsesWith(BB ? BB : DBB.OldBB);
> }
>
> - // We don't expect any of these to grow after clearing.
> - assert(DistinctWorklist.empty());
> + // We don't expect these to grow after clearing.
> assert(DelayedInits.empty());
> assert(DelayedBBs.empty());
> }
>
> Modified: llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp?rev=265456&r1=265455&r2=265456&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp (original)
> +++ llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp Tue Apr 5 15:23:21 2016
> @@ -16,6 +16,51 @@ using namespace llvm;
>
> namespace {
>
> +TEST(ValueMapperTest, MapMetadata) {
> + LLVMContext Context;
> + auto *U = MDTuple::get(Context, None);
> +
> + // The node should be unchanged.
> + ValueToValueMapTy VM;
> + EXPECT_EQ(U, MapMetadata(U, VM, RF_None));
> +}
> +
> +TEST(ValueMapperTest, MapMetadataCycle) {
> + LLVMContext Context;
> + MDNode *U0;
> + MDNode *U1;
> + {
> + Metadata *Ops[] = {nullptr};
> + auto T = MDTuple::getTemporary(Context, Ops);
> + Ops[0] = T.get();
> + U0 = MDTuple::get(Context, Ops);
> + T->replaceOperandWith(0, U0);
> + U1 = MDNode::replaceWithUniqued(std::move(T));
> + U0->resolveCycles();
> + }
> +
> + EXPECT_TRUE(U0->isResolved());
> + EXPECT_TRUE(U0->isUniqued());
> + EXPECT_TRUE(U1->isResolved());
> + EXPECT_TRUE(U1->isUniqued());
> + EXPECT_EQ(U1, U0->getOperand(0));
> + EXPECT_EQ(U0, U1->getOperand(0));
> +
> + // Cycles shouldn't be duplicated.
> + {
> + ValueToValueMapTy VM;
> + EXPECT_EQ(U0, MapMetadata(U0, VM, RF_None));
> + EXPECT_EQ(U1, MapMetadata(U1, VM, RF_None));
> + }
> +
> + // Check the other order.
> + {
> + ValueToValueMapTy VM;
> + EXPECT_EQ(U1, MapMetadata(U1, VM, RF_None));
> + EXPECT_EQ(U0, MapMetadata(U0, VM, RF_None));
> + }
> +}
> +
> TEST(ValueMapperTest, MapMetadataUnresolved) {
> LLVMContext Context;
> TempMDTuple T = MDTuple::getTemporary(Context, None);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> --
> Mike
> Sent from phone
More information about the llvm-commits
mailing list