[PATCH] D25241: [libcxx] Improve code generation for vector::clear().
This revision was automatically updated to reflect the committed changes.
Closed by commit rL298601: [libcxx] Improve code generation for
vector::clear(). (authored by brucem).
Changed prior to commit:
https://reviews.llvm.org/D25241?vs=73790&id=92796#toc
Repository:
rL LLVM
https://reviews.llvm.org/D25241
Files:
libcxx/trunk/include/vector
libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
Index: libcxx/trunk/include/vector
===
--- libcxx/trunk/include/vector
+++ libcxx/trunk/include/vector
@@ -413,8 +413,10 @@
void
__vector_base<_Tp, _Allocator>::__destruct_at_end(pointer __new_last) _NOEXCEPT
{
-while (__new_last != __end_)
-__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));
+pointer __soon_to_be_end = __end_;
+while (__new_last != __soon_to_be_end)
+__alloc_traits::destroy(__alloc(),
_VSTD::__to_raw_pointer(--__soon_to_be_end));
+__end_ = __new_last;
}
template
Index:
libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
===
---
libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
+++
libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
@@ -0,0 +1,40 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// void clear();
+
+#include
+#include
+
+#include "min_allocator.h"
+#include "asan_testing.h"
+
+int main()
+{
+{
+int a[] = {1, 2, 3};
+std::vector c(a, a+3);
+c.clear();
+assert(c.empty());
+LIBCPP_ASSERT(c.__invariants());
+LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
+}
+#if TEST_STD_VER >= 11
+{
+int a[] = {1, 2, 3};
+std::vector> c(a, a+3);
+c.clear();
+assert(c.empty());
+LIBCPP_ASSERT(c.__invariants());
+LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
+}
+#endif
+}
Index: libcxx/trunk/include/vector
===
--- libcxx/trunk/include/vector
+++ libcxx/trunk/include/vector
@@ -413,8 +413,10 @@
void
__vector_base<_Tp, _Allocator>::__destruct_at_end(pointer __new_last) _NOEXCEPT
{
-while (__new_last != __end_)
-__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));
+pointer __soon_to_be_end = __end_;
+while (__new_last != __soon_to_be_end)
+__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__soon_to_be_end));
+__end_ = __new_last;
}
template
Index: libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
===
--- libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
+++ libcxx/trunk/test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
@@ -0,0 +1,40 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// void clear();
+
+#include
+#include
+
+#include "min_allocator.h"
+#include "asan_testing.h"
+
+int main()
+{
+{
+int a[] = {1, 2, 3};
+std::vector c(a, a+3);
+c.clear();
+assert(c.empty());
+LIBCPP_ASSERT(c.__invariants());
+LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
+}
+#if TEST_STD_VER >= 11
+{
+int a[] = {1, 2, 3};
+std::vector> c(a, a+3);
+c.clear();
+assert(c.empty());
+LIBCPP_ASSERT(c.__invariants());
+LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
+}
+#endif
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25241: [libcxx] Improve code generation for vector::clear().
EricWF accepted this revision. EricWF added a comment. Still LGTM Comment at: test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp:18 +#include "min_allocator.h" + +int main() LIBCPP_ASSERT requires including test_macros.h https://reviews.llvm.org/D25241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25241: [libcxx] Improve code generation for vector::clear().
brucem added a comment. This was accepted long ago, and then I got sidetracked for a while. Is this still okay to land? https://reviews.llvm.org/D25241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25241: [libcxx] Improve code generation for vector::clear().
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D25241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25241: [libcxx] Improve code generation for vector::clear().
brucem marked 2 inline comments as done. brucem added a comment. These have been addressed, so this should be good for further review. https://reviews.llvm.org/D25241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25241: [libcxx] Improve code generation for vector::clear().
brucem updated this revision to Diff 73790.
brucem added a comment.
Address comments from mclow.
https://reviews.llvm.org/D25241
Files:
include/vector
test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
Index: test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
===
--- /dev/null
+++ test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
@@ -0,0 +1,39 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// void clear();
+
+#include
+#include
+
+#include "min_allocator.h"
+
+int main()
+{
+{
+int a[] = {1, 2, 3};
+std::vector c(a, a+3);
+c.clear();
+assert(c.empty());
+LIBCPP_ASSERT(c.__invariants());
+LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
+}
+#if TEST_STD_VER >= 11
+{
+int a[] = {1, 2, 3};
+std::vector> c(a, a+3);
+c.clear();
+assert(c.empty());
+LIBCPP_ASSERT(c.__invariants());
+LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
+}
+#endif
+}
Index: include/vector
===
--- include/vector
+++ include/vector
@@ -413,8 +413,10 @@
void
__vector_base<_Tp, _Allocator>::__destruct_at_end(pointer __new_last) _NOEXCEPT
{
-while (__new_last != __end_)
-__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));
+pointer __soon_to_be_end = __end_;
+while (__new_last != __soon_to_be_end)
+__alloc_traits::destroy(__alloc(),
_VSTD::__to_raw_pointer(--__soon_to_be_end));
+__end_ = __new_last;
}
template
Index: test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
===
--- /dev/null
+++ test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
@@ -0,0 +1,39 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// void clear();
+
+#include
+#include
+
+#include "min_allocator.h"
+
+int main()
+{
+{
+int a[] = {1, 2, 3};
+std::vector c(a, a+3);
+c.clear();
+assert(c.empty());
+LIBCPP_ASSERT(c.__invariants());
+LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
+}
+#if TEST_STD_VER >= 11
+{
+int a[] = {1, 2, 3};
+std::vector> c(a, a+3);
+c.clear();
+assert(c.empty());
+LIBCPP_ASSERT(c.__invariants());
+LIBCPP_ASSERT(is_contiguous_container_asan_correct(c));
+}
+#endif
+}
Index: include/vector
===
--- include/vector
+++ include/vector
@@ -413,8 +413,10 @@
void
__vector_base<_Tp, _Allocator>::__destruct_at_end(pointer __new_last) _NOEXCEPT
{
-while (__new_last != __end_)
-__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));
+pointer __soon_to_be_end = __end_;
+while (__new_last != __soon_to_be_end)
+__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__soon_to_be_end));
+__end_ = __new_last;
}
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25241: [libcxx] Improve code generation for vector::clear().
mclow.lists added a comment. I had no idea that we had no tests for `vector::clear`. Oosps. This looks good to me, but I want to play with the codegen for a bit before approving it. Also, you should add `cfe-commits` to the subscribers. thanks for doing this! > clear.pass.cpp:25 > +c.clear(); > +assert(c.empty()); > +} How about a couple more assertions here: LIBCPP_ASSERT(c.__invariants());` LIBCPP_ASSERT(is_contiguous_container_asan_correct(c)); > clear.pass.cpp:32 > +c.clear(); > +assert(c.empty()); > +} Same here. Repository: rL LLVM https://reviews.llvm.org/D25241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25241: [libcxx] Improve code generation for vector::clear().
brucem created this revision.
brucem added a subscriber: cfe-commits.
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
movq8(%rdi), %rcx
cmpq%rdx, %rcx
jeLBB2_2
leaq-12(%rcx), %rax
subq%rdx, %rax
movabsq$-6148914691236517205, %rdx ## imm = 0xAAAB
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.
https://reviews.llvm.org/D25241
Files:
include/vector
test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
Index: test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
===
--- /dev/null
+++ test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
@@ -0,0 +1,35 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// void clear();
+
+#include
+#include
+
+#include "min_allocator.h"
+
+int main()
+{
+{
+int a[] = {1, 2, 3};
+std::vector c(a, a+3);
+c.clear();
+assert(c.empty());
+}
+#if TEST_STD_VER >= 11
+{
+int a[] = {1, 2, 3};
+std::vector> c(a, a+3);
+c.clear();
+assert(c.empty());
+}
+#endif
+}
Index: include/vector
===
--- include/vector
+++ include/vector
@@ -413,8 +413,10 @@
void
__vector_base<_Tp, _Allocator>::__destruct_at_end(pointer __new_last) _NOEXCEPT
{
-while (__new_last != __end_)
-__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));
+pointer __soon_to_be_end = __end_;
+while (__new_last != __soon_to_be_end)
+__alloc_traits::destroy(__alloc(),
_VSTD::__to_raw_pointer(--__soon_to_be_end));
+__end_ = __new_last;
}
template
Index: test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
===
--- /dev/null
+++ test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp
@@ -0,0 +1,35 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+
+// void clear();
+
+#include
+#include
+
+#include "min_allocator.h"
+
+int main()
+{
+{
+int a[] = {1, 2, 3};
+std::vector c(a, a+3);
+c.clear();
+assert(c.empty());
+}
+#if TEST_STD_VER >= 11
+{
+int a[] = {1, 2, 3};
+std::vector> c(a, a+3);
+c.clear();
+assert(c.empty());
+}
+#endif
+}
Index: include/vector
===
--- include/vector
+++ include/vector
@@ -413,8 +413,10 @@
void
__vector_base<_Tp, _Allocator>::__destruct_at_end(pointer __new_last) _NOEXCEPT
{
-while (__new_last != __end_)
-__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__end_));
+pointer __soon_to_be_end = __end_;
+while (__new_last != __soon_to_be_end)
+__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(--__soon_to_be_end));
+__end_ = __new_last;
}
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
