[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:31:19 PDT 2016
r265470.
> On 2016-Apr-05, at 14:13, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
> The problem was un-inferred move constructors.
>
> If r265465 doesn't fix it, I'll spell them out the error-prone way.
>
>> On 2016-Apr-05, at 14:07, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> 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
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list