#
95529874 |
| 08-May-2016 |
Junmo Park <junmoz.park@samsung.com> |
Minor code cleanups. NFC.
llvm-svn: 268888
|
#
71480bd0 |
| 22-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper/Enumerator: Clean up code in post-order traversals, NFC
Re-layer the functions in the new (i.e., newly correct) post-order traversals in ValueEnumerator (r266947) and ValueMapper (r26694
ValueMapper/Enumerator: Clean up code in post-order traversals, NFC
Re-layer the functions in the new (i.e., newly correct) post-order traversals in ValueEnumerator (r266947) and ValueMapper (r266949). Instead of adding a node to the worklist in a helper function and returning a flag to say what happened, return the node itself. This makes the code way cleaner: the worklist is local to the main function, there is no flag for an early loop exit (since we can cleanly bury the loop), and it's perfectly clear when pointers into the worklist might be invalidated.
I'm fixing both algorithms in the same commit to avoid repeating the commit message; if you take the time to understand one the other should be easy. The diff itself isn't entirely obvious since the traversals have some noise (i.e., things to do), but here's the high-level change:
auto helper = [&WL](T *Op) { auto helper = [](T **&I, T **E) { => while (I != E) { if (shouldVisit(Op)) { T *Op = *I++; WL.push(Op, Op->begin()); if (shouldVisit(Op)) { return true; return Op; } } return false; return nullptr; }; }; => WL.push(S, S->begin()); WL.push(S, S->begin()); while (!empty()) { while (!empty()) { auto *N = WL.top().N; auto *N = WL.top().N; auto *&I = WL.top().I; auto *&I = WL.top().I; bool DidChange = false; while (I != N->end()) if (helper(*I++)) { => if (T *Op = helper(I, N->end()) { DidChange = true; WL.push(Op, Op->begin()); break; continue; } } if (DidChange) continue;
POT.push(WL.pop()); => POT.push(WL.pop()); } }
Thanks to Mehdi for helping me find a better way to layer this.
llvm-svn: 267099
show more ...
|
#
0ab44dbf |
| 21-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Map uniqued nodes in post-order
The iteratitive algorithm from r265456 claimed but failed to create a post-order traversal. It had the same error that was fixed in the ValueEnumerator
ValueMapper: Map uniqued nodes in post-order
The iteratitive algorithm from r265456 claimed but failed to create a post-order traversal. It had the same error that was fixed in the ValueEnumerator in r266947: now, instead of pushing all operands on the worklist at once, we pause whenever an operand gets pushed in order to go depth-first (I know, it sounds obvious).
Sadly, I have no idea how to observe this from outside the algorithm and so I haven't written a test. The output should be the same; it should just use fewer temporary nodes now. I've added some comments that I hope make the current logic clear enough it's unlikely to regress.
llvm-svn: 266949
show more ...
|
#
0fdaf8c9 |
| 17-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
Linker: Don't double-schedule appending variables
Add an assertion to ValueMapper that prevents double-scheduling of GlobalValues to remap, and fix the one place it happened. There are tons of test
Linker: Don't double-schedule appending variables
Add an assertion to ValueMapper that prevents double-scheduling of GlobalValues to remap, and fix the one place it happened. There are tons of tests that fail with this assertion in place and without the code change, so I'm not adding another.
Although it looks related, r266563 was, indeed, removing dead code. AFAICT, this cross-file double-scheduling started in r266510 when the cross-file recursion was removed.
llvm-svn: 266569
show more ...
|
#
3d555ac9 |
| 17-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Don't allow explicit null mappings of Values, NFC
As a follow-up to r123058, assert that there are no null mappings in the ValueMap instead of just ignoring them when they are there. T
ValueMapper: Don't allow explicit null mappings of Values, NFC
As a follow-up to r123058, assert that there are no null mappings in the ValueMap instead of just ignoring them when they are there. There were a couple of accidental insertions in CloneFunction so I cleaned those up (caught by testcases).
llvm-svn: 266565
show more ...
|
#
5ab2be09 |
| 17-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
IR: Use an explicit map for debug info type uniquing
Rather than relying on the structural equivalence of DICompositeType to merge type definitions, use an explicit map on the LLVMContext that LLPar
IR: Use an explicit map for debug info type uniquing
Rather than relying on the structural equivalence of DICompositeType to merge type definitions, use an explicit map on the LLVMContext that LLParser and BitcodeReader consult when constructing new nodes. Each non-forward-declaration DICompositeType with a non-empty 'identifier:' field is stored/loaded from the type map, and the first definiton will "win".
This map is opt-in: clients that expect ODR types from different modules to be merged must call LLVMContext::ensureDITypeMap.
- Clients that just happen to load more than one Module in the same LLVMContext won't magically merge types.
- Clients (like LTO) that want to continue to merge types based on ODR identifiers should opt-in immediately.
I have updated LTOCodeGenerator.cpp, the two "linking" spots in gold-plugin.cpp, and llvm-link (unless -disable-debug-info-type-map) to set this.
With this in place, it will be straightforward to remove the DITypeRef concept (i.e., referencing types by their 'identifier:' string rather than pointing at them directly).
llvm-svn: 266549
show more ...
|
#
694ab4e9 |
| 16-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Separate mapping of distinct and uniqued nodes (again)
Since the result of a mapped distinct node is known up front, it's more efficient to map them separately from uniqued nodes. This
ValueMapper: Separate mapping of distinct and uniqued nodes (again)
Since the result of a mapped distinct node is known up front, it's more efficient to map them separately from uniqued nodes. This commit pulls them out of the post-order traversal and stores them in a worklist to be remapped at the top-level.
This is essentially reapplying r244181 ("ValueMapper: Rotate distinct node remapping algorithm") to the new iterative algorithm from r265456 ("ValueMapper: Rewrite Mapper::mapMetadata without recursion").
Now that the traversal logic only handles uniqued MDNodes, it's much simpler to inline it all into MDNodeMapper::createPOT (I've killed the MDNodeMapper::push and MDNodeMapper::tryToPop helpers and localized the traversal worklist).
The resulting high-level algorithm for MDNodeMapper::map now looks like this:
- Distinct nodes are immediately mapped and added to MDNodeMapper::DistinctWorklist using MDNodeMapper::mapDistinctNode.
- Uniqued nodes are mapped via MDNodeMapper::mapTopLevelUniquedNode, which traverses the transitive uniqued subgraph of a node to calculate uniqued node mappings in bulk.
- This is a simplified version of MDNodeMapper::map from before this commit (originally r265456) that doesn't traverse through any distinct nodes.
- Distinct nodes are added to MDNodeMapper::DistinctWorklist via MDNodeMapper::mapDistinctNode.
- This uses MDNodeMapper::createPOT to fill a MDNodeMapper::UniquedGraph (a post-order traversal and side table), UniquedGraph::propagateChanges to track which uniqued nodes need to change, and MDNodeMapper::mapNodesInPOT to create the uniqued nodes.
- Placeholders for forward references are now only needed when there's a uniquing cycle (a cycle of uniqued nodes unbroken by distinct nodes). This is the key functionality change that we're reintroducing (from r244181). As of r265456, a temporary forward reference might be needed for any cycle that involved uniqued nodes.
- After mapping the first node appropriately, MDNodeMapper::map works through MDNodeMapper::DistinctWorklist. For each distinct node, its operands are remapped with MDNodeMapper::mapDistinctNode and MDNodeMapper::mapTopLevelUniquedNode until all nodes have been mapped.
Sadly there's nothing observable I can test here; no real functionality change, just a compile-time speedup from reduced malloc traffic.
llvm-svn: 266537
show more ...
|
#
0cb5c344 |
| 16-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Only put cyclic nodes into CyclicNodes, NFCI
As a minor fixup to r266258, only track nodes that needed a placeholder in CyclicNodes in MDNodeMapper::mapUniquedNodes. There should be no
ValueMapper: Only put cyclic nodes into CyclicNodes, NFCI
As a minor fixup to r266258, only track nodes that needed a placeholder in CyclicNodes in MDNodeMapper::mapUniquedNodes. There should be no observable functionality change, just some local memory savings because CyclicNodes only needs to grow to accommodate nodes that are actually involved in cycles. (This was the original intent of r266258, or else the vector would have been called "ChangedNodes".)
llvm-svn: 266536
show more ...
|
#
e12bef7e |
| 16-Apr-2016 |
Simon Atanasyan <simon@atanasyan.com> |
ValueMapper: Fix unused var warning. NFC
llvm-svn: 266529
|
#
a77d0733 |
| 16-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Stop memoizing ConstantAsMetadata
Stop memoizing ConstantAsMetadata in ValueMapper::mapMetadata. Now we have to recompute it, but these metadata aren't particularly common, and it rest
ValueMapper: Stop memoizing ConstantAsMetadata
Stop memoizing ConstantAsMetadata in ValueMapper::mapMetadata. Now we have to recompute it, but these metadata aren't particularly common, and it restricts the lifetime of the Metadata map unnecessarily.
(The motivation is that I have a patch which uses a single Metadata map for the lifetime of IRMover. Mehdi profiled r266446 with the patch applied and we saw a pretty big speedup in lib/Linker.)
llvm-svn: 266513
show more ...
|
#
39423b02 |
| 16-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
Reapply "ValueMapper: Eliminate cross-file co-recursion, NFC"
This reverts commit r266507, reapplying r266503 (and r266505 "ValueMapper: Use API from r266503 in unit tests, NFC") completely unchange
Reapply "ValueMapper: Eliminate cross-file co-recursion, NFC"
This reverts commit r266507, reapplying r266503 (and r266505 "ValueMapper: Use API from r266503 in unit tests, NFC") completely unchanged.
I reverted because of a bot failure here: http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/16810/
However, looking more closely, the failure was from a host-compiler crash (clang 3.7.1) when building: lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/DwarfAccelTable.cpp.o
I didn't modify that file, or anything it includes, with that commit.
The next build (which hadn't picked up my revert) got past it: http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/16811/
I think this was just unfortunate timing. I suppose the bot must be flakey.
llvm-svn: 266510
show more ...
|
#
6fe1ff26 |
| 16-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
Revert "ValueMapper: Eliminate cross-file co-recursion, NFC"
This reverts commit r266503, in case it's the root cause of this bot failure:
http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/bui
Revert "ValueMapper: Eliminate cross-file co-recursion, NFC"
This reverts commit r266503, in case it's the root cause of this bot failure:
http://lab.llvm.org:8011/builders/lld-x86_64-freebsd/builds/16810
I'm also reverting r266505 -- "ValueMapper: Use API from r266503 in unit tests, NFC" -- since it's in the way.
llvm-svn: 266507
show more ...
|
#
f0d73f95 |
| 16-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Eliminate cross-file co-recursion, NFC
Eliminate co-recursion of Mapper::mapValue through ValueMaterializer::materializeInitFor, through a major redesign of the ValueMapper.cpp interfac
ValueMapper: Eliminate cross-file co-recursion, NFC
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. Long-term, 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 hold-op is that we need to deprecate C API that relies on this.
Secondly, IRLinker has special logic to correctly implement aliases with comdats, and uses two ValueToValueMapTy instances and two ValueMaterializers. I supported this by allowing clients to register an alternate mapping context, whose MCID can be passed in when scheduling new work.
While out of scope for this commit, it should now be straightforward to remove recursion from Mapper::mapValue.
llvm-svn: 266503
show more ...
|
#
db6861e7 |
| 15-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Hide Mapper::VM behind an accessor, NFC
Change Mapper::VM to a pointer and add a `getVM()` accessor for it. While this has no functionality change, it minimizes the diff on an upcoming
ValueMapper: Hide Mapper::VM behind an accessor, NFC
Change Mapper::VM to a pointer and add a `getVM()` accessor for it. While this has no functionality change, it minimizes the diff on an upcoming patch that allows switching between instances of ValueToValueMapTy on a single Mapper instance.
llvm-svn: 266490
show more ...
|
#
96d2a1c6 |
| 14-Apr-2016 |
Davide Italiano <davide@freebsd.org> |
[ValueMapper] Range-loopify to improve readability. NFC.
llvm-svn: 266350
|
#
11f60fd6 |
| 13-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Resolve cycles on the new nodes
Fix a major bug from r265456. Although it's now much rarer, ValueMapper sometimes has to duplicate cycles. The might-transitively-reference-a-temporary
ValueMapper: Resolve cycles on the new nodes
Fix a major bug from r265456. Although it's now much rarer, ValueMapper sometimes has to duplicate cycles. The might-transitively-reference-a-temporary counts don't decrement on their own when there are cycles, and you need to call MDNode::resolveCycles to fix it.
r265456 was checking the input nodes to see if they were unresolved. This is useless; they should never be unresolved. Instead we should check the output nodes and resolve cycles on them.
llvm-svn: 266258
show more ...
|
#
bb2c3e19 |
| 08-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Extract llvm::RemapFunction from IRMover.cpp, NFC
Strip out the remapping parts of IRLinker::linkFunctionBody and put them in ValueMapper.cpp under the name Mapper::remapFunction (with
ValueMapper: Extract llvm::RemapFunction from IRMover.cpp, NFC
Strip out the remapping parts of IRLinker::linkFunctionBody and put them in ValueMapper.cpp under the name Mapper::remapFunction (with a top-level entry-point llvm::RemapFunction).
This is a nice cleanup on its own since it puts the remapping code together and shares a single Mapper context for the entire IRLinker::linkFunctionBody Call. Besides that, this will make it easier to break the co-recursion between IRMover.cpp and ValueMapper.cpp in follow ups.
llvm-svn: 265835
show more ...
|
#
adcebdf2 |
| 08-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Always use Mapper::mapValue from remapInstruction, NFCI
Use Mapper::mapValue instead of llvm::MapValue from Mapper::remapInstruction when mapping an incoming block for a PHINode (follow
ValueMapper: Always use Mapper::mapValue from remapInstruction, NFCI
Use Mapper::mapValue instead of llvm::MapValue from Mapper::remapInstruction when mapping an incoming block for a PHINode (follow-up to r265832). This will implicitly pass along the Materializer argument, but when this code was added in r133513 there was no Materializer argument. I suspect this call to MapValue was just missed in r182776 since it's not observable (basic blocks can't be materialized, and they don't reference other values).
llvm-svn: 265833
show more ...
|
#
a574e7a7 |
| 08-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Roll RemapInstruction into Mapper, NFC
Add Mapper::remapInstruction, move the guts of llvm::RemapInstruction into it, and use the same Mapper for most of the calls to MapValue and MapMe
ValueMapper: Roll RemapInstruction into Mapper, NFC
Add Mapper::remapInstruction, move the guts of llvm::RemapInstruction into it, and use the same Mapper for most of the calls to MapValue and MapMetadata. There should be no functionality change here.
I left off the call to MapValue that wasn't passing in a Materializer argument (for basic blocks of PHINodes). It shouldn't change functionality either, but I'm suspicious enough to commit separately.
llvm-svn: 265832
show more ...
|
#
69341e6a |
| 08-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Don't memoize metadata when RF_NoModuleLevelChanges
Prevent the Metadata side-table in ValueMap from growing unnecessarily when RF_NoModuleLevelChanges. As a drive-by, make ValueMap::h
ValueMapper: Don't memoize metadata when RF_NoModuleLevelChanges
Prevent the Metadata side-table in ValueMap from growing unnecessarily when RF_NoModuleLevelChanges. As a drive-by, make ValueMap::hasMD, which apparently had no users until I used it here for testing, actually compile.
llvm-svn: 265828
show more ...
|
#
e05ff7c1 |
| 08-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Stop memoizing MDStrings
Stop adding MDString to the Metadata section of the ValueMap in MapMetadata. It blows up the size of the map for no benefit, since we can always return quickly
ValueMapper: Stop memoizing MDStrings
Stop adding MDString to the Metadata section of the ValueMap in MapMetadata. It blows up the size of the map for no benefit, since we can always return quickly anyway.
There is a potential follow-up that I don't think I'll push on right away, but maybe someone else is interested: stop checking for a pre-mapped MDString, and move the `isa<MDString>()` checks in Mapper::mapSimpleMetadata and MDNodeMapper::getMappedOp in front of the `VM.getMappedMD()` calls. While this would preclude explicitly remapping MDStrings it would probably be a little faster.
llvm-svn: 265827
show more ...
|
#
4ec55f8a |
| 08-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
Reapply "ValueMapper: Treat LocalAsMetadata more like function-local Values"
This reverts commit r265765, reapplying r265759 after changing a call from LocalAsMetadata::get to ValueAsMetadata::get (
Reapply "ValueMapper: Treat LocalAsMetadata more like function-local Values"
This reverts commit r265765, reapplying r265759 after changing a call from LocalAsMetadata::get to ValueAsMetadata::get (and adding a unit test). When a local value is mapped to a constant (like "i32 %a" => "i32 7"), the new debug intrinsic operand may no longer be pointing at a local.
http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_build/19020/
The previous coommit message follows:
--
This is a partial re-commit -- maybe more of a re-implementation -- of r265631 (reverted in r265637).
This makes RF_IgnoreMissingLocals behave (almost) consistently between the Value and the Metadata hierarchy. In particular:
- MapValue returns nullptr or "metadata !{}" for missing locals in MetadataAsValue/LocalAsMetadata bridging paris, depending on the RF_IgnoreMissingLocals flag.
- MapValue doesn't memoize LocalAsMetadata-related results.
- MapMetadata no longer deals with LocalAsMetadata or RF_IgnoreMissingLocals at all. (This wasn't in r265631 at all, but I realized during testing it would make the patch simpler with no loss of generality.)
r265631 went too far, making both functions universally ignore RF_IgnoreMissingLocals. This broke building (e.g.) compiler-rt. Reassociate (and possibly other passes) don't currently maintain dominates-use invariants for metadata operands, resulting in IR like this:
define void @foo(i32 %arg) { call void @llvm.some.intrinsic(metadata i32 %x) %x = add i32 1, i32 %arg }
If the inliner chooses to inline @foo into another function, then RemapInstruction will call `MapValue(metadata i32 %x)` and assert that the return is not nullptr.
I've filed PR27273 to add a Verifier check and fix the underlying problem in the optimization passes.
As a workaround, return `!{}` instead of nullptr for unmapped LocalAsMetadata when RF_IgnoreMissingLocals is unset. Otherwise, match the behaviour of r265631.
Original commit message:
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: 265768
show more ...
|
#
80587314 |
| 08-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
Revert "ValueMapper: Treat LocalAsMetadata more like function-local Values"
This reverts commit r265759, since even this limited version breaks some bots: http://lab.llvm.org:8011/builders/clang-p
Revert "ValueMapper: Treat LocalAsMetadata more like function-local Values"
This reverts commit r265759, since even this limited version breaks some bots: http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3311 http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/17696
This also reverts r265761 "ValueMapper: Unduplicate RF_NoModuleLevelChanges check, NFC", since I had trouble separating it from r265759.
llvm-svn: 265765
show more ...
|
#
0fa8aca0 |
| 08-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Unduplicate RF_NoModuleLevelChanges check, NFC
llvm-svn: 265761
|
#
267185ec |
| 08-Apr-2016 |
Duncan P. N. Exon Smith <dexonsmith@apple.com> |
ValueMapper: Treat LocalAsMetadata more like function-local Values
This is a partial re-commit -- maybe more of a re-implementation -- of r265631 (reverted in r265637).
This makes RF_IgnoreMissingL
ValueMapper: Treat LocalAsMetadata more like function-local Values
This is a partial re-commit -- maybe more of a re-implementation -- of r265631 (reverted in r265637).
This makes RF_IgnoreMissingLocals behave (almost) consistently between the Value and the Metadata hierarchy. In particular:
- MapValue returns nullptr or "metadata !{}" for missing locals in MetadataAsValue/LocalAsMetadata bridging paris, depending on the RF_IgnoreMissingLocals flag.
- MapValue doesn't memoize LocalAsMetadata-related results.
- MapMetadata no longer deals with LocalAsMetadata or RF_IgnoreMissingLocals at all. (This wasn't in r265631 at all, but I realized during testing it would make the patch simpler with no loss of generality.)
r265631 went too far, making both functions universally ignore RF_IgnoreMissingLocals. This broke building (e.g.) compiler-rt. Reassociate (and possibly other passes) don't currently maintain dominates-use invariants for metadata operands, resulting in IR like this:
define void @foo(i32 %arg) { call void @llvm.some.intrinsic(metadata i32 %x) %x = add i32 1, i32 %arg }
If the inliner chooses to inline @foo into another function, then RemapInstruction will call `MapValue(metadata i32 %x)` and assert that the return is not nullptr.
I've filed PR27273 to add a Verifier check and fix the underlying problem in the optimization passes.
As a workaround, return `!{}` instead of nullptr for unmapped LocalAsMetadata when RF_IgnoreMissingLocals is unset. Otherwise, match the behaviour of r265631.
Original commit message:
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: 265759
show more ...
|