[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:15:09 PDT 2016
> On 2016-Apr-20, at 14:12, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
>>
>> 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.
If I *also* comment out the TypedTrackingMDRef copy constructor and assignment, this compiles with std::vector. But it looks to me like std::vector (at least the version I'm using) prefers the copying versions.
> 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