[llvm] r266909 - IR: Use SmallVector instead of std::vector of TrackingMDRef
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 20 14:12:50 PDT 2016
> On 2016-Apr-20, at 13:59, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>>
>> On 2016-Apr-20, at 13:28, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Wed, Apr 20, 2016 at 1:14 PM, Duncan P. N. Exon Smith via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> Author: dexonsmith
>> Date: Wed Apr 20 15:14:09 2016
>> New Revision: 266909
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=266909&view=rev
>> Log:
>> IR: Use SmallVector instead of std::vector of TrackingMDRef
>>
>> Don't use std::vector<TrackingMDRef>, since (at least in some versions
>> of libc++) std::vector apparently copies values on grow operations
>> instead of moving them. Found this when I was temporarily deleting the
>> copy constructor for TrackingMDRef to investigate a performance
>> bottleneck.
>>
>> Got a build log? That seems unlikely, since we have vectors of unique_ptr in the codebase. Perhaps you used some specific operation that required a copy?
It may be that it uses copy if available; otherwise move.
>> I imagine the append you had in the previous patch? Should've been an append with move_iterators? (& maybe that should've been 'assign' instead, I think - in passing)
>
> It's the `emplace_back()` calls.
Copied in build log for the one in DIBuilder:
>>
>> Modified:
>> llvm/trunk/include/llvm/IR/DIBuilder.h
>> llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
>> llvm/trunk/lib/IR/DIBuilder.cpp
>>
>> Modified: llvm/trunk/include/llvm/IR/DIBuilder.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/DIBuilder.h?rev=266909&r1=266908&r2=266909&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/DIBuilder.h (original)
>> +++ llvm/trunk/include/llvm/IR/DIBuilder.h Wed Apr 20 15:14:09 2016
>> @@ -51,7 +51,11 @@ namespace llvm {
>> bool AllowUnresolvedNodes;
>>
>> /// Each subprogram's preserved local variables.
>> - DenseMap<MDNode *, std::vector<TrackingMDNodeRef>> PreservedVariables;
>> + ///
>> + /// Do not use a std::vector. Some versions of libc++ apparently copy
>> + /// instead of move on grow operations, and TrackingMDRef is expensive to
>> + /// copy.
>> + DenseMap<MDNode *, SmallVector<TrackingMDNodeRef, 1>> PreservedVariables;
>>
>> DIBuilder(const DIBuilder &) = delete;
>> void operator=(const DIBuilder &) = delete;
>>
>> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=266909&r1=266908&r2=266909&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
>> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Wed Apr 20 15:14:09 2016
>> @@ -107,7 +107,12 @@ class BitcodeReaderMetadataList {
>> bool AnyFwdRefs;
>> unsigned MinFwdRef;
>> unsigned MaxFwdRef;
>> - std::vector<TrackingMDRef> MetadataPtrs;
>> +
>> + /// Array of metadata references.
>> + ///
>> + /// Don't use std::vector here. Some versions of libc++ copy (instead of
>> + /// move) on resize, and TrackingMDRef is very expensive to copy.
>> + SmallVector<TrackingMDRef, 1> MetadataPtrs;
>>
>> LLVMContext &Context;
>> public:
>>
>> Modified: llvm/trunk/lib/IR/DIBuilder.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DIBuilder.cpp?rev=266909&r1=266908&r2=266909&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/IR/DIBuilder.cpp (original)
>> +++ llvm/trunk/lib/IR/DIBuilder.cpp Wed Apr 20 15:14:09 2016
>> @@ -614,7 +614,7 @@ DIGlobalVariable *DIBuilder::createTempG
>>
>> static DILocalVariable *createLocalVariable(
>> LLVMContext &VMContext,
>> - DenseMap<MDNode *, std::vector<TrackingMDNodeRef>> &PreservedVariables,
>> + DenseMap<MDNode *, SmallVector<TrackingMDNodeRef, 1>> &PreservedVariables,
>> DIScope *Scope, StringRef Name, unsigned ArgNo, DIFile *File,
>> unsigned LineNo, DIType *Ty, bool AlwaysPreserve, unsigned Flags) {
>> // FIXME: Why getNonCompileUnitScope()?
The code is:
--
PreservedVariables[Fn].emplace_back(Node);
--
Commenting out TrackingMDRef copy constructor and assignment, this compiles with SmallVector.
With std::vector, I get:
--
$ /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/IR -I/Users/dexonsmith/data/llvm/staging/lib/IR -Iinclude -I/Users/dexonsmith/data/llvm/staging/include -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -fcolor-diagnostics -O2 -g -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk -UNDEBUG -fno-exceptions -fno-rtti -MMD -MT lib/IR/CMakeFiles/LLVMCore.dir/DIBuilder.cpp.o -MF lib/IR/CMakeFiles/LLVMCore.dir/DIBuilder.cpp.o.d -o lib/IR/CMakeFiles/LLVMCore.dir/DIBuilder.cpp.o -c /Users/dexonsmith/data/llvm/staging/lib/IR/DIBuilder.cpp
In file included from /Users/dexonsmith/data/llvm/staging/lib/IR/DIBuilder.cpp:14:
In file included from /Users/dexonsmith/data/llvm/staging/include/llvm/IR/DIBuilder.h:19:
/Users/dexonsmith/data/llvm/staging/include/llvm/IR/TrackingMDRef.h:108:53: error: call to implicitly-deleted copy constructor of 'llvm::TrackingMDRef'
TypedTrackingMDRef(const TypedTrackingMDRef &X) : Ref(X.Ref) {}
^ ~~~~~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:1731:31: note: in instantiation of member function
'llvm::TypedTrackingMDRef<llvm::MDNode>::TypedTrackingMDRef' requested here
::new((void*)__p) _Up(_VSTD::forward<_Args>(__args)...);
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:1647:18: note: in instantiation of function template specialization
'std::__1::allocator<llvm::TypedTrackingMDRef<llvm::MDNode> >::construct<llvm::TypedTrackingMDRef<llvm::MDNode>, const llvm::TypedTrackingMDRef<llvm::MDNode> &>' requested here
{__a.construct(__p, _VSTD::forward<_Args>(__args)...);}
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:1493:14: note: in instantiation of function template specialization
'std::__1::allocator_traits<std::__1::allocator<llvm::TypedTrackingMDRef<llvm::MDNode> > >::__construct<llvm::TypedTrackingMDRef<llvm::MDNode>, const llvm::TypedTrackingMDRef<llvm::MDNode>
&>' requested here
{__construct(__has_construct<allocator_type, _Tp*, _Args...>(),
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/memory:1609:17: note: in instantiation of function template specialization
'std::__1::allocator_traits<std::__1::allocator<llvm::TypedTrackingMDRef<llvm::MDNode> > >::construct<llvm::TypedTrackingMDRef<llvm::MDNode>, const llvm::TypedTrackingMDRef<llvm::MDNode>
&>' requested here
construct(__a, _VSTD::__to_raw_pointer(__end2-1), _VSTD::move_if_noexcept(*--__end1));
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:889:21: note: in instantiation of function template specialization
'std::__1::allocator_traits<std::__1::allocator<llvm::TypedTrackingMDRef<llvm::MDNode> > >::__construct_backward<llvm::TypedTrackingMDRef<llvm::MDNode> *>' requested here
__alloc_traits::__construct_backward(this->__alloc(), this->__begin_, this->__end_, __v.__begin_);
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:1627:5: note: in instantiation of member function
'std::__1::vector<llvm::TypedTrackingMDRef<llvm::MDNode>, std::__1::allocator<llvm::TypedTrackingMDRef<llvm::MDNode> > >::__swap_out_circular_buffer' requested here
__swap_out_circular_buffer(__v);
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/vector:1646:9: note: in instantiation of function template specialization
'std::__1::vector<llvm::TypedTrackingMDRef<llvm::MDNode>, std::__1::allocator<llvm::TypedTrackingMDRef<llvm::MDNode> > >::__emplace_back_slow_path<llvm::DILocalVariable *&>' requested here
__emplace_back_slow_path(_VSTD::forward<_Args>(__args)...);
^
/Users/dexonsmith/data/llvm/staging/lib/IR/DIBuilder.cpp:635:28: note: in instantiation of function template specialization 'std::__1::vector<llvm::TypedTrackingMDRef<llvm::MDNode>,
std::__1::allocator<llvm::TypedTrackingMDRef<llvm::MDNode> > >::emplace_back<llvm::DILocalVariable *&>' requested here
PreservedVariables[Fn].emplace_back(Node);
^
/Users/dexonsmith/data/llvm/staging/include/llvm/IR/TrackingMDRef.h:31:3: note: copy constructor is implicitly deleted because 'TrackingMDRef' has a user-declared move constructor
TrackingMDRef(TrackingMDRef &&X) : MD(X.MD) { retrack(X); }
^
1 error generated.
--
More information about the llvm-commits
mailing list