exceptions are disabled.
The patch was reverted due to some confusion about non-movable types. ie
types
that explicitly delete their move constructors. However, such types do
not meet
the requirement for `MoveConstructible`, which is required by
`std::vector`:
Summary:
`std::vector<T>` is free choose between using copy or move operations
when it
needs to resize. The standard only candidates that the correct exception
safety
guarantees are provided. When exceptions are disabled these guarantees
are
trivially satisfied. Meaning vector is free to optimize it's
implementation by
moving instead of copying.
This patch makes `std::vector` unconditionally move elements when
exceptions are
disabled. This optimization is conforming according to the current
standard wording.
There are concerns that moving in `-fno-noexceptions`mode will be a
surprise to
users. For example, a user may be surprised to find their code is slower
with
exceptions enabled than it is disabled. I'm sympathetic to this
surprised, but
I don't think it should block this optimization.
Reviewers: mclow.lists, ldionne, rsmith
Reviewed By: ldionne
Subscribers: zoecarver, christof, dexonsmith, libcxx-commits
Tags: #libc
Differential Revision: https://reviews.llvm.org/D62228
llvm-svn: 371867
This reverts r370502, which broke the use case of a copy-only T (with a
deleted move constructor) when exceptions are disabled. Until we figure
out the right behavior, I'm reverting the commit.
llvm-svn: 371068
Summary:
`std::vector<T>` is free choose between using copy or move operations when it needs to resize. The standard only candidates that the correct exception safety guarantees are provided. When exceptions are disabled these guarantees are trivially satisfied. Meaning vector is free to optimize it's implementation by moving instead of copying.
This patch makes `std::vector` unconditionally move elements when exceptions are disabled.
This optimization is conforming according to the current standard wording.
There are concerns that moving in `-fno-noexceptions`mode will be a surprise to users. For example, a user may be surprised to find their code is slower with exceptions enabled than it is disabled. I'm sympathetic to this surprised, but I don't think it should block this optimization.
Reviewers: mclow.lists, ldionne, rsmith
Reviewed By: ldionne
Subscribers: zoecarver, christof, dexonsmith, libcxx-commits
Tags: #libc
Differential Revision: https://reviews.llvm.org/D62228
llvm-svn: 370502
Since we build the library with -fvisibility=hidden, the shared object
wouldn't contain __vector_base_common<true>::__throw_length_error()
and __vector_base_common<true>::__throw_out_of_range(), leading to
link errors. This only happened on GCC for some reason.
https://llvm.org/PR43140
llvm-svn: 370240
The optimizer is petulant and temperamental. In this case LLVM failed to lower
the the "insert at end" loop used by`vector<unsigned char>` to a `memset` despite
`memset` being substantially faster over a range of bytes.
LLVM has the ability to lower loops to `memset` whet appropriate, but the
odd nature of libc++'s loops prevented the optimization from taking places.
This patch addresses the issue by rewriting the loops from the form
`do [ ... --__n; } while (__n > 0);` to instead use a for loop over a pointer
range (For example: `for (auto *__i = ...; __i < __e; ++__i)`).
This patch also rewrites the asan annotations to unposion all additional memory
at the start of the loop instead of once per iterations. This could potentially
permit false negatives where the constructor of element N attempts to access
element N + 1 during its construction.
The before and after results for the `BM_ConstructSize/vector_byte/5140480_mean`
benchmark (run 5 times) are:
--------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations
--------------------------------------------------------------------------------------------
Before
------
BM_ConstructSize/vector_byte/5140480_mean 12530140 ns 12469693 ns N/A
BM_ConstructSize/vector_byte/5140480_median 12512818 ns 12445571 ns N/A
BM_ConstructSize/vector_byte/5140480_stddev 106224 ns 107907 ns 5
-----
After
-----
BM_ConstructSize/vector_byte/5140480_mean 167285 ns 166500 ns N/A
BM_ConstructSize/vector_byte/5140480_median 166749 ns 166069 ns N/A
BM_ConstructSize/vector_byte/5140480_stddev 3242 ns 3184 ns 5
llvm-svn: 367183
Summary:
The reason libc++ implemented a throwing debug mode handler was for ease of testing. Specifically,
I thought that if a debug violation aborted, we could only test one violation per file. This made
it impossible to test debug mode. Which throwing behavior we could test more!
However, the throwing approach didn't work either, since there are debug violations underneath noexcept
functions. This lead to the introduction of `_NOEXCEPT_DEBUG`, which was only noexcept when debug
mode was off.
Having thought more and having grown wiser, `_NOEXCEPT_DEBUG` was a horrible decision. It was
viral, it didn't cover all the cases it needed to, and it was observable to the user -- at worst
changing the behavior of their program.
This patch removes the throwing debug handler, and rewrites the debug tests using 'fork-ing' style
death tests.
Reviewers: mclow.lists, ldionne, thomasanderson
Reviewed By: ldionne
Subscribers: christof, arphaman, libcxx-commits, #libc
Differential Revision: https://reviews.llvm.org/D59166
llvm-svn: 356417
to reflect the new license. These used slightly different spellings that
defeated my regular expressions.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351648
Summary:
We never actually mean to always inline a function -- all the uses of
the macro I could find are actually attempts to control the visibility
of symbols. This is better described by _LIBCPP_INLINE_VISIBILITY, which
is actually always defined the same.
This change is orthogonal to the decision of what we're actually going
to do with _LIBCPP_INLINE_VISIBILITY -- it just simplifies things by
having one canonical way of doing things.
Note that this commit had originally been applied in r336369 and then
reverted in r336382 because of unforeseen problems. Both of these problems
have now been fixed.
Reviewers: EricWF, mclow.lists
Subscribers: christof, dexonsmith, erikvanderpoel
Differential Revision: https://reviews.llvm.org/D48892
llvm-svn: 336866
This reverts commit r336369. The commit had two problems:
1. __pbump was marked as _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY instead of
_LIBCPP_INLINE_VISIBILITY, which lead to two symbols being added in the
dylib and the check-cxx-abilist failing.
2. The LLDB tests started failing because they undefine
`_LIBCPP_INLINE_VISIBILITY`. I need to figure out why they do that and
fix the tests before we can go forward with this change.
llvm-svn: 336382
Summary:
We never actually mean to always inline a function -- all the uses of
the macro I could find are actually attempts to control the visibility
of symbols. This is better described by _LIBCPP_INLINE_VISIBILITY, which
is actually always defined the same.
This change is orthogonal to the decision of what we're actually going
to do with _LIBCPP_INLINE_VISIBILITY -- it just simplifies things by
having one canonical way of doing things.
Reviewers: EricWF
Subscribers: christof, llvm-commits, dexonsmith, erikvanderpoel, mclow.lists
Differential Revision: https://reviews.llvm.org/D48892
llvm-svn: 336369
C++2a[container.requirements.general]p8 states that when move constructing
a container, the allocator is move constructed. Vector previously copy
constructed these allocators. This patch fixes that bug.
Additionally it cleans up some unnecessary allocator conversions
when copy constructing containers. Libc++ uses
__internal_allocator_traits::select_on_copy_construction to select
the correct allocator during copy construction, but it unnecessarily
converted the resulting allocator to the user specified allocator
type and back. After this patch list and forward_list no longer
do that.
Technically we're supposed to be using allocator_traits<allocator_type>::select_on_copy_construction,
but that should seemingly be addressed as a separate patch, if at all.
llvm-svn: 334053
Summary:
The constructors `vector(Iter, Iter, Alloc = Alloc{})` and `assign(Iter, Iter)` don't correctly perform EmplaceConstruction from the result of dereferencing the iterator. This results in them performing an additional and unneeded copy.
This patch addresses the issue by correctly using `emplace_back` in C++11 and newer.
There are also some bugs in our `insert` implementation, but those will be handled separately.
@mclow.lists We should probably merge this into 5.1, agreed?
Reviewers: mclow.lists, dlj, EricWF
Reviewed By: mclow.lists, EricWF
Subscribers: cfe-commits, mclow.lists
Differential Revision: https://reviews.llvm.org/D38757
llvm-svn: 315994
Once upon a time, extern templates used to be a Microsoft extension, so
cl would warn about their usage, and libc++ suppressed that warning.
They've long since been standardized, so the warning is defunct. (libc++
also doesn't currently support building with cl anyway.)
llvm-svn: 307997
Summary:
This patch improves how libc++ handles min/max macros within the headers. Previously libc++ would undef them and emit a warning.
This patch changes libc++ to use `#pragma push_macro` to save the macro before undefining it, and `#pragma pop_macro` to restore the macros and the end of the header.
Reviewers: mclow.lists, bcraig, compnerd, EricWF
Reviewed By: EricWF
Subscribers: cfe-commits, krytarowski
Differential Revision: https://reviews.llvm.org/D33080
llvm-svn: 304357
This patch cleans up all usages of the following feature test macros inside
<vector> and its tests:
* _LIBCPP_HAS_NO_RVALUE_REFERENCES
* _LIBCPP_HAS_NO_VARIADICS
* _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS
Where needed the above guards were replaced with _LIBCPP_CXX03_LANG.
llvm-svn: 300410
Summary:
By manipulating a local variable in the loop, when the loop can
be optimized away (due to no non-trivial destructors), this lets
it be fully optimized away and we modify the __end_ separately.
This results in a substantial improvement in the generated code.
Prior to this change, this would be generated (on x86_64):
movq (%rdi), %rdx
movq 8(%rdi), %rcx
cmpq %rdx, %rcx
je LBB2_2
leaq -12(%rcx), %rax
subq %rdx, %rax
movabsq $-6148914691236517205, %rdx ## imm = 0xAAAAAAAAAAAAAAAB
mulq %rdx
shrq $3, %rdx
notq %rdx
leaq (%rdx,%rdx,2), %rax
leaq (%rcx,%rax,4), %rax
movq %rax, 8(%rdi)
And after:
movq (%rdi), %rax
movq %rax, 8(%rdi)
This brings this in line with what other implementations do.
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D25241
llvm-svn: 298601
The name _LIBCPP_TYPE_VIS_ONLY is no longer accurate because both
_LIBCPP_TYPE_VIS and _LIBCPP_TYPE_VIS_ONLY expand to
__attribute__((__type_visibility__)) with Clang. The only remaining difference
is that _LIBCPP_TYPE_VIS_ONLY can be applied to templates whereas
_LIBCPP_TYPE_VIS cannot (due to dllimport/dllexport not being allowed on
templates).
This patch renames _LIBCPP_TYPE_VIS_ONLY to _LIBCPP_TEMPLATE_VIS.
llvm-svn: 291035
Summary: The `max_size()` method of containers should respect both the allocator's reported `max_size` and the range of the `difference_type`. This patch makes all containers choose the smallest of those two values.
Reviewers: mclow.lists, EricWF
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D26885
llvm-svn: 287729
Summary:
This patch fixes a number of problems with the visibility macros across GCC (on Unix) and Windows (DLL import/export semantics). All of the visibility macros are now documented under `DesignDocs/VisibilityMacros.rst`. Now I'll no longer forget the subtleties of each!
This patch adds two new visibility macros:
* `_LIBCPP_ENUM_VIS` for controlling the typeinfo of enum types. Only Clang supports this.
* `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` for redefining visibility on explicit instantiation declarations. Clang and Windows require this.
After applying this patch GCC only emits one -Wattribute warning opposed to 30+.
Reviewers: mclow.lists, EricWF
Subscribers: beanz, mgorny, cfe-commits
Differential Revision: https://reviews.llvm.org/D24602
llvm-svn: 281673
Summary:
This bug was originally fixed in http://reviews.llvm.org/D7201.
However it was broken again by the fix to https://llvm.org/bugs/show_bug.cgi?id=22605.
This patch re-fixes __wrap_iter with GCC by providing a forward declaration of <vector> before the friend declaration in __wrap_iter.
This patch avoids the issues in PR22605 by putting canonical forward declarations in <iosfwd> and including <iosfwd> in <vector>.
<iosfwd> was chosen as the canonical forward declaration headers for the following reasons:
1. `<iosfwd>` is small with almost no dependancies.
2. It already forward declares `std::allocator`
3. It is already included in `<iterator>` which we need to fix the GCC bug.
This patch fixes the test "gcc_workaround.pass.cpp"
Reviewers: mclow.lists, EricWF
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D16345
llvm-svn: 261382
This change moves visibility attributes from out-of-class method
definitions to in-class declaration. This is needed for a switch to
attribute((internal_linkage)) (see http://reviews.llvm.org/D13925)
which can only appear on the first declaration.
This change does not touch istream/ostream/streambuf. They are
handled separately in http://reviews.llvm.org/D14409.
llvm-svn: 252385
Summary:
In certain cases vector can use memcpy to construct a range of elements at the back of the vector. We currently don't do this resulting in terrible code gen in non-optimized mode and a
very large slowdown compared to libstdc++.
This patch adds a `__construct_forward_range(Allocator, Iter, Iter, _Ptr&)` and `__construct_forward_range(Allocator, Tp*, Tp*, Tp*&)` functions to `allocator_traits` which act similarly to the existing `__construct_forward(...)` functions.
This patch also changes vectors `__construct_at_end(Iter, Iter)` to be `__construct_at_end(Iter, Iter, SizeType)` where SizeType is the size of the range. `__construct_at_end(Iter, Iter, SizeType)` now calls `allocator_traits<Tp>::__construct_forward_range(...)`.
This patch is based off the design of `__swap_out_circular_buffer(...)` which uses `allocator_traits<Tp>::__construct_forward(...)`.
On my machine this code performs 4x better than the current implementation when tested against `std::vector<int>`.
Reviewers: howard.hinnant, titus, kcc, mclow.lists
Reviewed By: mclow.lists
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D8109
llvm-svn: 233711
Summary: Fix suggested by @mclow.lists on D8109. Store the size of the un-poisoned vector upon construction instead of calculating it later.
Reviewers: titus, mclow.lists, kcc, EricWF
Reviewed By: EricWF
Subscribers: mclow.lists, cfe-commits
Differential Revision: http://reviews.llvm.org/D8172
llvm-svn: 231729
Summary:
The size of the vector is being increased by `__n` during the call to `__move_range` and not by 1.
This fixes a test failure in `containers/sequences/vector/vector.modifiers/insert_iter_size_value.pass.cpp` when using ASAN.
Reviewers: danalbert, kcc, mclow.lists
Reviewed By: mclow.lists
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D6264
llvm-svn: 222014