<div dir="ltr">It seems that this changes breaks sanitizer-windows bot:<div><br></div><div><a href="http://lab.llvm.org:8011/builders/sanitizer-windows/builds/19822/steps/run%20tests/logs/stdio">http://lab.llvm.org:8011/builders/sanitizer-windows/builds/19822/steps/run%20tests/logs/stdio</a><br></div><div><br></div><div><div>[162/1466] Building CXX object lib\LTO\CMakeFiles\LLVMLTO.dir\LTOCodeGenerator.cpp.obj</div><div>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</div><div>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</div><div> 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'</div><div> This diagnostic occurred in the compiler generated function '`anonymous-namespace'::MDNodeMapper::Data::Data(const `anonymous-namespace'::MDNodeMapper::Data &)'</div><div>ninja: build stopped: subcommand failed.</div></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Apr 5, 2016 at 1:28 PM Duncan P. N. Exon Smith via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dexonsmith<br>
Date: Tue Apr 5 15:23:21 2016<br>
New Revision: 265456<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=265456&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=265456&view=rev</a><br>
Log:<br>
ValueMapper: Rewrite Mapper::mapMetadata without recursion<br>
<br>
This commit completely rewrites Mapper::mapMetadata (the implementation<br>
of llvm::MapMetadata) using an iterative algorithm. The guts of the new<br>
algorithm are in MDNodeMapper::map, the entry function in a new class.<br>
<br>
Previously, Mapper::mapMetadata performed a recursive exploration of the<br>
graph with eager "just in case there's a reason" malloc traffic.<br>
<br>
The new algorithm has these benefits:<br>
<br>
- New nodes and temporaries are not created eagerly.<br>
- Uniquing cycles are not duplicated (see new unit test).<br>
- No recursion.<br>
<br>
Given a node to map, it does this:<br>
<br>
1. Use a worklist to perform a post-order traversal of the transitively<br>
referenced unmapped nodes.<br>
<br>
2. Track which nodes will change operands, and which will have new<br>
addresses in the mapped scheme. Propagate the changes through the<br>
POT until fixed point, to pick up uniquing cycles that need to<br>
change.<br>
<br>
3. Map all the distinct nodes without touching their operands. If<br>
RF_MoveDistinctMetadata, they get mapped to themselves; otherwise,<br>
they get mapped to clones.<br>
<br>
4. Map the uniqued nodes (bottom-up), lazily creating temporaries for<br>
forward references as needed.<br>
<br>
5. Remap the operands of the distinct nodes.<br>
<br>
Mehdi helped me out by profiling this with -flto=thin. On his workload<br>
(importing/etc. for opt.cpp), MapMetadata sped up by 15%, contributed<br>
about 50% less to persistent memory, and made about 100x fewer calls to<br>
malloc. The speedup is less than I'd hoped. The profile mainly blames<br>
DenseMap lookups; perhaps there's a way to reduce them (e.g., by<br>
disallowing remapping of MDString).<br>
<br>
It would be nice to break the strange remaining recursion on the Value<br>
side: MapValue => materializeInitFor => RemapInstruction => MapValue. I<br>
think we could do this by having materializeInitFor return a worklist of<br>
things to be remapped.<br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp<br>
llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=265456&r1=265455&r2=265456&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=265456&r1=265455&r2=265456&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Tue Apr 5 15:23:21 2016<br>
@@ -58,7 +58,10 @@ struct DelayedBasicBlock {<br>
TempBB(BasicBlock::Create(Old.getContext())) {}<br>
};<br>
<br>
+class MDNodeMapper;<br>
class Mapper {<br>
+ friend class MDNodeMapper;<br>
+<br>
ValueToValueMapTy &VM;<br>
RemapFlags Flags;<br>
ValueMapTypeRemapper *TypeMapper;<br>
@@ -66,7 +69,6 @@ class Mapper {<br>
<br>
SmallVector<DelayedGlobalValueInit, 8> DelayedInits;<br>
SmallVector<DelayedBasicBlock, 1> DelayedBBs;<br>
- SmallVector<MDNode *, 8> DistinctWorklist;<br>
<br>
public:<br>
Mapper(ValueToValueMapTy &VM, RemapFlags Flags,<br>
@@ -87,39 +89,146 @@ public:<br>
private:<br>
Value *mapBlockAddress(const BlockAddress &BA);<br>
<br>
- /// Map metadata helper.<br>
- ///<br>
- /// Co-recursively finds the mapping for MD. If this returns an MDNode, it's<br>
- /// possible that MDNode::isResolved() will return false.<br>
- Metadata *mapMetadataImpl(const Metadata *MD);<br>
- Metadata *mapMetadataOp(Metadata *Op);<br>
-<br>
/// Map metadata that doesn't require visiting operands.<br>
Optional<Metadata *> mapSimpleMetadata(const Metadata *MD);<br>
<br>
- /// Remap the operands of an MDNode.<br>
+ Metadata *mapToMetadata(const Metadata *Key, Metadata *Val);<br>
+ Metadata *mapToSelf(const Metadata *MD);<br>
+};<br>
+<br>
+class MDNodeMapper {<br>
+ Mapper &M;<br>
+<br>
+ struct Data {<br>
+ bool HasChangedOps = false;<br>
+ bool HasChangedAddress = false;<br>
+ unsigned ID = ~0u;<br>
+ TempMDNode Placeholder;<br>
+ };<br>
+<br>
+ SmallDenseMap<const Metadata *, Data, 32> Info;<br>
+ SmallVector<std::pair<MDNode *, bool>, 16> Worklist;<br>
+ SmallVector<MDNode *, 16> POT;<br>
+<br>
+public:<br>
+ MDNodeMapper(Mapper &M) : M(M) {}<br>
+<br>
+ /// Map a metadata node (and its transitive operands).<br>
///<br>
- /// If \c Node is temporary, uniquing cycles are ignored. If \c Node is<br>
- /// distinct, uniquing cycles are resolved as they're found.<br>
+ /// This is the only entry point into MDNodeMapper. It works as follows:<br>
///<br>
- /// \pre \c Node.isDistinct() or \c Node.isTemporary().<br>
- bool remapOperands(MDNode &Node);<br>
+ /// 1. \a createPOT(): use a worklist to perform a post-order traversal of<br>
+ /// the transitively referenced unmapped nodes.<br>
+ ///<br>
+ /// 2. \a propagateChangedOperands(): track which nodes will change<br>
+ /// operands, and which will have new addresses in the mapped scheme.<br>
+ /// Propagate the changes through the POT until fixed point, to pick up<br>
+ /// uniquing cycles that need to change.<br>
+ ///<br>
+ /// 3. \a mapDistinctNodes(): map all the distinct nodes without touching<br>
+ /// their operands. If RF_MoveDistinctMetadata, they get mapped to<br>
+ /// themselves; otherwise, they get mapped to clones.<br>
+ ///<br>
+ /// 4. \a mapUniquedNodes(): map the uniqued nodes (bottom-up), lazily<br>
+ /// creating temporaries for forward references as needed.<br>
+ ///<br>
+ /// 5. \a remapDistinctOperands(): remap the operands of the distinct nodes.<br>
+ Metadata *map(const MDNode &FirstN);<br>
+<br>
+private:<br>
+ /// Return \c true as long as there's work to do.<br>
+ bool hasWork() const { return !Worklist.empty(); }<br>
<br>
- /// Map a distinct MDNode.<br>
+ /// Get the current node in the worklist.<br>
+ MDNode &getCurrentNode() const { return *Worklist.back().first; }<br>
+<br>
+ /// Push a node onto the worklist.<br>
+ ///<br>
+ /// Adds \c N to \a Worklist and \a Info, unless it's already inserted. If<br>
+ /// \c N.isDistinct(), \a Data::HasChangedAddress will be set based on \a<br>
+ /// RF_MoveDistinctMDs.<br>
+ ///<br>
+ /// Returns the data for the node.<br>
///<br>
- /// Whether distinct nodes change is independent of their operands. If \a<br>
- /// RF_MoveDistinctMDs, then they are reused, and their operands remapped in<br>
- /// place; effectively, they're moved from one graph to another. Otherwise,<br>
- /// they're cloned/duplicated, and the new copy's operands are remapped.<br>
- Metadata *mapDistinctNode(const MDNode *Node);<br>
+ /// \post Data::HasChangedAddress iff !RF_MoveDistinctMDs && N.isDistinct().<br>
+ /// \post Worklist.back().first == &N.<br>
+ /// \post Worklist.back().second == false.<br>
+ Data &push(const MDNode &N);<br>
<br>
- /// Map a uniqued MDNode.<br>
+ /// Map a node operand, and return true if it changes.<br>
///<br>
- /// Uniqued nodes may not need to be recreated (they may map to themselves).<br>
- Metadata *mapUniquedNode(const MDNode *Node);<br>
+ /// \post getMappedOp(Op) does not return None.<br>
+ bool mapOperand(const Metadata *Op);<br>
<br>
- Metadata *mapToMetadata(const Metadata *Key, Metadata *Val);<br>
- Metadata *mapToSelf(const Metadata *MD);<br>
+ /// Get a previously mapped node.<br>
+ Optional<Metadata *> getMappedOp(const Metadata *Op) const;<br>
+<br>
+ /// Try to pop a node off the worklist and store it in POT.<br>
+ ///<br>
+ /// Returns \c true if it popped; \c false if its operands need to be<br>
+ /// visited.<br>
+ ///<br>
+ /// \post If Worklist.back().second == false: Worklist.back().second == true.<br>
+ /// \post Else: Worklist.back() has been popped off and added to \a POT.<br>
+ bool tryToPop();<br>
+<br>
+ /// Get a forward reference to a node to use as an operand.<br>
+ ///<br>
+ /// Returns \c Op if it's not changing; otherwise, lazily creates a temporary<br>
+ /// node and returns it.<br>
+ Metadata &getFwdReference(const Data &D, MDNode &Op);<br>
+<br>
+ /// Create a post-order traversal from the given node.<br>
+ ///<br>
+ /// This traverses the metadata graph deeply enough to map \c FirstN. It<br>
+ /// uses \a mapOperand() (indirectly, \a Mapper::mapSimplifiedNode()), so any<br>
+ /// metadata that has already been mapped will not be part of the POT.<br>
+ ///<br>
+ /// \post \a POT is a post-order traversal ending with \c FirstN.<br>
+ bool createPOT(const MDNode &FirstN);<br>
+<br>
+ /// Propagate changed operands through post-order traversal.<br>
+ ///<br>
+ /// Until fixed point, iteratively update:<br>
+ ///<br>
+ /// - \a Data::HasChangedOps based on \a Data::HasChangedAddress of operands;<br>
+ /// - \a Data::HasChangedAddress based on Data::HasChangedOps.<br>
+ ///<br>
+ /// This algorithm never changes \a Data::HasChangedAddress for distinct<br>
+ /// nodes.<br>
+ ///<br>
+ /// \post \a POT is a post-order traversal ending with \c FirstN.<br>
+ void propagateChangedOperands();<br>
+<br>
+ /// Map all distinct nodes in POT.<br>
+ ///<br>
+ /// \post \a getMappedOp() returns the correct node for every distinct node.<br>
+ void mapDistinctNodes();<br>
+<br>
+ /// Map all uniqued nodes in POT with the correct operands.<br>
+ ///<br>
+ /// \pre Distinct nodes are mapped (\a mapDistinctNodes() has been called).<br>
+ /// \post \a getMappedOp() returns the correct node for every node.<br>
+ /// \post \a MDNode::operands() is correct for every uniqued node.<br>
+ /// \post \a MDNode::isResolved() returns true for every node.<br>
+ void mapUniquedNodes();<br>
+<br>
+ /// Re-map the operands for distinct nodes in POT.<br>
+ ///<br>
+ /// \pre Distinct nodes are mapped (\a mapDistinctNodes() has been called).<br>
+ /// \pre Uniqued nodes are mapped (\a mapUniquedNodes() has been called).<br>
+ /// \post \a MDNode::operands() is correct for every distinct node.<br>
+ void remapDistinctOperands();<br>
+<br>
+ /// Remap a node's operands.<br>
+ ///<br>
+ /// Iterate through operands and update them in place using \a getMappedOp()<br>
+ /// and \a getFwdReference().<br>
+ ///<br>
+ /// \pre N.isDistinct() or N.isTemporary().<br>
+ /// \pre Distinct nodes are mapped (\a mapDistinctNodes() has been called).<br>
+ /// \pre If \c N is distinct, all uniqued nodes are already mapped.<br>
+ void remapOperands(const Data &D, MDNode &N);<br>
};<br>
<br>
} // end namespace<br>
@@ -280,78 +389,218 @@ Metadata *Mapper::mapToSelf(const Metada<br>
return mapToMetadata(MD, const_cast<Metadata *>(MD));<br>
}<br>
<br>
-Metadata *Mapper::mapMetadataOp(Metadata *Op) {<br>
+bool MDNodeMapper::mapOperand(const Metadata *Op) {<br>
+ if (!Op)<br>
+ return false;<br>
+<br>
+ if (Optional<Metadata *> MappedOp = M.mapSimpleMetadata(Op)) {<br>
+ assert(M.VM.getMappedMD(Op) && "Expected result to be memoized");<br>
+ return *MappedOp != Op;<br>
+ }<br>
+<br>
+ return push(*cast<MDNode>(Op)).HasChangedAddress;<br>
+}<br>
+<br>
+Optional<Metadata *> MDNodeMapper::getMappedOp(const Metadata *Op) const {<br>
if (!Op)<br>
return nullptr;<br>
<br>
- if (Metadata *MappedOp = mapMetadataImpl(Op))<br>
- return MappedOp;<br>
- // Use identity map if MappedOp is null and we can ignore missing entries.<br>
- if (Flags & RF_IgnoreMissingEntries)<br>
+ if (Optional<Metadata *> MappedOp = M.VM.getMappedMD(Op))<br>
+ return *MappedOp;<br>
+<br>
+ return None;<br>
+}<br>
+<br>
+Metadata &MDNodeMapper::getFwdReference(const Data &D, MDNode &Op) {<br>
+ auto Where = Info.find(&Op);<br>
+ assert(Where != Info.end() && "Expected a valid reference");<br>
+<br>
+ auto &OpD = Where->second;<br>
+ assert(OpD.ID > <a href="http://D.ID" rel="noreferrer" target="_blank">D.ID</a> && "Expected a forward reference");<br>
+<br>
+ if (!OpD.HasChangedAddress)<br>
return Op;<br>
<br>
- return nullptr;<br>
+ // Lazily construct a temporary node.<br>
+ if (!OpD.Placeholder)<br>
+ OpD.Placeholder = Op.clone();<br>
+<br>
+ return *OpD.Placeholder;<br>
+}<br>
+<br>
+void MDNodeMapper::remapOperands(const Data &D, MDNode &N) {<br>
+ for (unsigned I = 0, E = N.getNumOperands(); I != E; ++I) {<br>
+ Metadata *Old = N.getOperand(I);<br>
+ Metadata *New;<br>
+ if (Optional<Metadata *> MappedOp = getMappedOp(Old)){<br>
+ New = *MappedOp;<br>
+ } else {<br>
+ assert(!N.isDistinct() &&<br>
+ "Expected all nodes to be pre-mapped for distinct operands");<br>
+ MDNode &OldN = *cast<MDNode>(Old);<br>
+ assert(!OldN.isDistinct() && "Expected distinct nodes to be pre-mapped");<br>
+ New = &getFwdReference(D, OldN);<br>
+ }<br>
+<br>
+ if (Old != New)<br>
+ N.replaceOperandWith(I, New);<br>
+ }<br>
}<br>
<br>
-/// Resolve uniquing cycles involving the given metadata.<br>
-static void resolveCycles(Metadata *MD) {<br>
- if (auto *N = dyn_cast_or_null<MDNode>(MD))<br>
- if (!N->isResolved())<br>
- N->resolveCycles();<br>
+MDNodeMapper::Data &MDNodeMapper::push(const MDNode &N) {<br>
+ auto Insertion = Info.insert(std::make_pair(&N, Data()));<br>
+ auto &D = Insertion.first->second;<br>
+ if (!Insertion.second)<br>
+ return D;<br>
+<br>
+ // Add to the worklist; check for distinct nodes that are required to be<br>
+ // copied.<br>
+ Worklist.push_back(std::make_pair(&const_cast<MDNode &>(N), false));<br>
+ D.HasChangedAddress = !(M.Flags & RF_MoveDistinctMDs) && N.isDistinct();<br>
+ return D;<br>
+}<br>
+<br>
+bool MDNodeMapper::tryToPop() {<br>
+ if (!Worklist.back().second) {<br>
+ Worklist.back().second = true;<br>
+ return false;<br>
+ }<br>
+<br>
+ MDNode *N = Worklist.pop_back_val().first;<br>
+ Info[N].ID = POT.size();<br>
+ POT.push_back(N);<br>
+ return true;<br>
+}<br>
+<br>
+bool MDNodeMapper::createPOT(const MDNode &FirstN) {<br>
+ bool AnyChanges = false;<br>
+<br>
+ // Do a traversal of the unmapped subgraph, tracking whether operands change.<br>
+ // In some cases, these changes will propagate naturally, but<br>
+ // propagateChangedOperands() catches the general case.<br>
+ AnyChanges |= push(FirstN).HasChangedAddress;<br>
+ while (hasWork()) {<br>
+ if (tryToPop())<br>
+ continue;<br>
+<br>
+ MDNode &N = getCurrentNode();<br>
+ bool LocalChanges = false;<br>
+ for (const Metadata *Op : N.operands())<br>
+ LocalChanges |= mapOperand(Op);<br>
+<br>
+ if (!LocalChanges)<br>
+ continue;<br>
+<br>
+ AnyChanges = true;<br>
+ auto &D = Info[&N];<br>
+ D.HasChangedOps = true;<br>
+<br>
+ // Uniqued nodes change address when operands change.<br>
+ if (!N.isDistinct())<br>
+ D.HasChangedAddress = true;<br>
+ }<br>
+ return AnyChanges;<br>
+}<br>
+<br>
+void MDNodeMapper::propagateChangedOperands() {<br>
+ bool AnyChangedAddresses;<br>
+ do {<br>
+ AnyChangedAddresses = false;<br>
+ for (MDNode *N : POT) {<br>
+ auto &NI = Info[N];<br>
+ if (NI.HasChangedOps)<br>
+ continue;<br>
+<br>
+ if (!llvm::any_of(N->operands(), [&](const Metadata *Op) {<br>
+ auto Where = Info.find(Op);<br>
+ return Where != Info.end() && Where->second.HasChangedAddress;<br>
+ }))<br>
+ continue;<br>
+<br>
+ NI.HasChangedOps = true;<br>
+ if (!N->isDistinct()) {<br>
+ NI.HasChangedAddress = true;<br>
+ AnyChangedAddresses = true;<br>
+ }<br>
+ }<br>
+ } while (AnyChangedAddresses);<br>
+}<br>
+<br>
+void MDNodeMapper::mapDistinctNodes() {<br>
+ // Map all the distinct nodes in POT.<br>
+ for (MDNode *N : POT) {<br>
+ if (!N->isDistinct())<br>
+ continue;<br>
+<br>
+ if (M.Flags & RF_MoveDistinctMDs)<br>
+ M.mapToSelf(N);<br>
+ else<br>
+ M.mapToMetadata(N, MDNode::replaceWithDistinct(N->clone()));<br>
+ }<br>
}<br>
<br>
-bool Mapper::remapOperands(MDNode &Node) {<br>
- assert(!Node.isUniqued() && "Expected temporary or distinct node");<br>
- const bool IsDistinct = Node.isDistinct();<br>
-<br>
- bool AnyChanged = false;<br>
- for (unsigned I = 0, E = Node.getNumOperands(); I != E; ++I) {<br>
- Metadata *Old = Node.getOperand(I);<br>
- Metadata *New = mapMetadataOp(Old);<br>
- if (Old != New) {<br>
- AnyChanged = true;<br>
- Node.replaceOperandWith(I, New);<br>
-<br>
- // Resolve uniquing cycles underneath distinct nodes on the fly so they<br>
- // don't infect later operands.<br>
- if (IsDistinct)<br>
- resolveCycles(New);<br>
+void MDNodeMapper::mapUniquedNodes() {<br>
+ // Construct uniqued nodes, building forward references as necessary.<br>
+ for (auto *N : POT) {<br>
+ if (N->isDistinct())<br>
+ continue;<br>
+<br>
+ auto &D = Info[N];<br>
+ assert(D.HasChangedAddress == D.HasChangedOps &&<br>
+ "Uniqued nodes should change address iff ops change");<br>
+ if (!D.HasChangedAddress) {<br>
+ M.mapToSelf(N);<br>
+ continue;<br>
}<br>
+<br>
+ TempMDNode ClonedN = D.Placeholder ? std::move(D.Placeholder) : N->clone();<br>
+ remapOperands(D, *ClonedN);<br>
+ M.mapToMetadata(N, MDNode::replaceWithUniqued(std::move(ClonedN)));<br>
}<br>
<br>
- return AnyChanged;<br>
+ // Resolve cycles.<br>
+ for (auto *N : POT)<br>
+ if (!N->isResolved())<br>
+ N->resolveCycles();<br>
}<br>
<br>
-Metadata *Mapper::mapDistinctNode(const MDNode *Node) {<br>
- assert(Node->isDistinct() && "Expected distinct node");<br>
+void MDNodeMapper::remapDistinctOperands() {<br>
+ for (auto *N : POT) {<br>
+ if (!N->isDistinct())<br>
+ continue;<br>
<br>
- MDNode *NewMD;<br>
- if (Flags & RF_MoveDistinctMDs)<br>
- NewMD = const_cast<MDNode *>(Node);<br>
- else<br>
- NewMD = MDNode::replaceWithDistinct(Node->clone());<br>
+ auto &D = Info[N];<br>
+ if (!D.HasChangedOps)<br>
+ continue;<br>
<br>
- // Remap operands later.<br>
- DistinctWorklist.push_back(NewMD);<br>
- return mapToMetadata(Node, NewMD);<br>
+ assert(D.HasChangedAddress == !bool(M.Flags & RF_MoveDistinctMDs) &&<br>
+ "Distinct nodes should change address iff they cannot be moved");<br>
+ remapOperands(D, D.HasChangedAddress ? *cast<MDNode>(*getMappedOp(N)) : *N);<br>
+ }<br>
}<br>
<br>
-Metadata *Mapper::mapUniquedNode(const MDNode *Node) {<br>
- assert(Node->isUniqued() && "Expected uniqued node");<br>
+Metadata *MDNodeMapper::map(const MDNode &FirstN) {<br>
+ assert(!(M.Flags & RF_NoModuleLevelChanges) &&<br>
+ "MDNodeMapper::map assumes module-level changes");<br>
+ assert(POT.empty() && "MDNodeMapper::map is not re-entrant");<br>
<br>
- // Create a temporary node and map it upfront in case we have a uniquing<br>
- // cycle. If necessary, this mapping will get updated by RAUW logic before<br>
- // returning.<br>
- auto ClonedMD = Node->clone();<br>
- mapToMetadata(Node, ClonedMD.get());<br>
- if (!remapOperands(*ClonedMD)) {<br>
- // No operands changed, so use the original.<br>
- ClonedMD->replaceAllUsesWith(const_cast<MDNode *>(Node));<br>
- return const_cast<MDNode *>(Node);<br>
- }<br>
+ // Require resolved nodes whenever metadata might be remapped.<br>
+ assert(FirstN.isResolved() && "Unexpected unresolved node");<br>
<br>
- // Uniquify the cloned node.<br>
- return MDNode::replaceWithUniqued(std::move(ClonedMD));<br>
+ // Return early if nothing at all changed.<br>
+ if (!createPOT(FirstN)) {<br>
+ for (const MDNode *N : POT)<br>
+ M.mapToSelf(N);<br>
+ return &const_cast<MDNode &>(FirstN);<br>
+ }<br>
+<br>
+ propagateChangedOperands();<br>
+ mapDistinctNodes();<br>
+ mapUniquedNodes();<br>
+ remapDistinctOperands();<br>
+<br>
+ // Return the original node, remapped.<br>
+ return *getMappedOp(&FirstN);<br>
}<br>
<br>
Optional<Metadata *> Mapper::mapSimpleMetadata(const Metadata *MD) {<br>
@@ -389,21 +638,6 @@ Optional<Metadata *> Mapper::mapSimpleMe<br>
return None;<br>
}<br>
<br>
-Metadata *Mapper::mapMetadataImpl(const Metadata *MD) {<br>
- assert(VM.mayMapMetadata() && "Unexpected co-recursion through mapValue");<br>
- if (Optional<Metadata *> NewMD = mapSimpleMetadata(MD))<br>
- return *NewMD;<br>
-<br>
- // Require resolved nodes whenever metadata might be remapped.<br>
- auto *Node = cast<MDNode>(MD);<br>
- assert(Node->isResolved() && "Unexpected unresolved node");<br>
-<br>
- if (Node->isDistinct())<br>
- return mapDistinctNode(Node);<br>
-<br>
- return mapUniquedNode(Node);<br>
-}<br>
-<br>
Metadata *llvm::MapMetadata(const Metadata *MD, ValueToValueMapTy &VM,<br>
RemapFlags Flags, ValueMapTypeRemapper *TypeMapper,<br>
ValueMaterializer *Materializer) {<br>
@@ -411,25 +645,13 @@ Metadata *llvm::MapMetadata(const Metada<br>
}<br>
<br>
Metadata *Mapper::mapMetadata(const Metadata *MD) {<br>
- Metadata *NewMD = mapMetadataImpl(MD);<br>
-<br>
- // When there are no module-level changes, it's possible that the metadata<br>
- // graph has temporaries. Skip the logic to resolve cycles, since it's<br>
- // unnecessary (and invalid) in that case.<br>
- if (Flags & RF_NoModuleLevelChanges)<br>
- return NewMD;<br>
-<br>
- // Resolve cycles involving the entry metadata.<br>
- resolveCycles(NewMD);<br>
+ if (Optional<Metadata *> NewMD = mapSimpleMetadata(MD))<br>
+ return *NewMD;<br>
<br>
- return NewMD;<br>
+ return MDNodeMapper(*this).map(*cast<MDNode>(MD));<br>
}<br>
<br>
Mapper::~Mapper() {<br>
- // Remap the operands of distinct MDNodes.<br>
- while (!DistinctWorklist.empty())<br>
- remapOperands(*DistinctWorklist.pop_back_val());<br>
-<br>
// Materialize global initializers.<br>
while (!DelayedInits.empty()) {<br>
auto Init = DelayedInits.pop_back_val();<br>
@@ -443,8 +665,7 @@ Mapper::~Mapper() {<br>
DBB.TempBB->replaceAllUsesWith(BB ? BB : DBB.OldBB);<br>
}<br>
<br>
- // We don't expect any of these to grow after clearing.<br>
- assert(DistinctWorklist.empty());<br>
+ // We don't expect these to grow after clearing.<br>
assert(DelayedInits.empty());<br>
assert(DelayedBBs.empty());<br>
}<br>
<br>
Modified: llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp?rev=265456&r1=265455&r2=265456&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp?rev=265456&r1=265455&r2=265456&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp (original)<br>
+++ llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp Tue Apr 5 15:23:21 2016<br>
@@ -16,6 +16,51 @@ using namespace llvm;<br>
<br>
namespace {<br>
<br>
+TEST(ValueMapperTest, MapMetadata) {<br>
+ LLVMContext Context;<br>
+ auto *U = MDTuple::get(Context, None);<br>
+<br>
+ // The node should be unchanged.<br>
+ ValueToValueMapTy VM;<br>
+ EXPECT_EQ(U, MapMetadata(U, VM, RF_None));<br>
+}<br>
+<br>
+TEST(ValueMapperTest, MapMetadataCycle) {<br>
+ LLVMContext Context;<br>
+ MDNode *U0;<br>
+ MDNode *U1;<br>
+ {<br>
+ Metadata *Ops[] = {nullptr};<br>
+ auto T = MDTuple::getTemporary(Context, Ops);<br>
+ Ops[0] = T.get();<br>
+ U0 = MDTuple::get(Context, Ops);<br>
+ T->replaceOperandWith(0, U0);<br>
+ U1 = MDNode::replaceWithUniqued(std::move(T));<br>
+ U0->resolveCycles();<br>
+ }<br>
+<br>
+ EXPECT_TRUE(U0->isResolved());<br>
+ EXPECT_TRUE(U0->isUniqued());<br>
+ EXPECT_TRUE(U1->isResolved());<br>
+ EXPECT_TRUE(U1->isUniqued());<br>
+ EXPECT_EQ(U1, U0->getOperand(0));<br>
+ EXPECT_EQ(U0, U1->getOperand(0));<br>
+<br>
+ // Cycles shouldn't be duplicated.<br>
+ {<br>
+ ValueToValueMapTy VM;<br>
+ EXPECT_EQ(U0, MapMetadata(U0, VM, RF_None));<br>
+ EXPECT_EQ(U1, MapMetadata(U1, VM, RF_None));<br>
+ }<br>
+<br>
+ // Check the other order.<br>
+ {<br>
+ ValueToValueMapTy VM;<br>
+ EXPECT_EQ(U1, MapMetadata(U1, VM, RF_None));<br>
+ EXPECT_EQ(U0, MapMetadata(U0, VM, RF_None));<br>
+ }<br>
+}<br>
+<br>
TEST(ValueMapperTest, MapMetadataUnresolved) {<br>
LLVMContext Context;<br>
TempMDTuple T = MDTuple::getTemporary(Context, None);<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><div dir="ltr">-- <br></div>Mike<br>Sent from phone