<div dir="ltr">Howard,<div>Whats preventing you from making that change. It looks better anyway. :)</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 2, 2015 at 1:42 AM, Howard Hinnant via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Oct 1, 2015, at 6:28 PM, chen xu via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br>
><br>
> Hi, my project code (linux, Clang as compiler and LLVM libcxx as c++ library) will have the execution of x=std::move(x) in some code path (not directly, but sometimes it will happen via levels of function calls and parameter-pass by reference, etc.), and it turns out x is cleared after this x = std::move(x). I tried gnu c++ library, and msvc c++ library (on windows), both keep x untouched.<br>
><br>
> I am wondering if this could be fixed in LLVM libcxx, as it seems making more sense not to clear x when moving x to itself. Thanks. Below is a sample code for repro. Thanks a lot.<br>
><br>
> #include <iostream><br>
> using namespace std;<br>
><br>
> class Test {<br>
> public:<br>
> Test() : x("test") { }<br>
> void MoveX() {<br>
> cout << "before x=move(x), x : " << x;<br>
> x=move(x);<br>
> cout << "after x=move(x), x : " << x;<br>
> }<br>
> private:<br>
> string x;<br>
> };<br>
><br>
> int main(int argc, char** argv)<br>
> {<br>
> Test test;<br>
> test.MoveX();<br>
> return 0;<br>
> }<br>
><br>
> The output would be like<br>
><br>
> before x=move(x), x : test<br>
> after x=move(x), x :<br>
<br>
</div></div>I would like to make the opposite suggestion. I think the libc++ string move assignment operator is too slow instead of too fast.<br>
<br>
Currently __move_assign(basic_string& __str, true_type) is coded like this:<br>
<br>
template <class _CharT, class _Traits, class _Allocator><br>
inline _LIBCPP_INLINE_VISIBILITY<br>
void<br>
basic_string<_CharT, _Traits, _Allocator>::__move_assign(basic_string& __str, true_type)<br>
#if _LIBCPP_STD_VER > 14<br>
_NOEXCEPT<br>
#else<br>
_NOEXCEPT_(is_nothrow_move_assignable<allocator_type>::value)<br>
#endif<br>
{<br>
clear();<br>
shrink_to_fit();<br>
__r_.first() = __str.__r_.first();<br>
__move_assign_alloc(__str);<br>
__str.__zero();<br>
}<br>
<br>
I think the author of this code was just being lazy (or maybe was too rushed). Using this test, at -O3:<br>
<br>
void<br>
test(std::string& s0, std::string&& s1)<br>
{<br>
s0 = std::move(s1);<br>
}<br>
<br>
this compiles down to:<br>
<br>
.section __TEXT,__text,regular,pure_instructions<br>
.macosx_version_min 10, 10<br>
.globl __Z4testRNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEOS5_<br>
.align 4, 0x90<br>
__Z4testRNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEOS5_: ## @_Z4testRNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEOS5_<br>
Lfunc_begin0:<br>
.cfi_startproc<br>
.cfi_personality 155, ___gxx_personality_v0<br>
.cfi_lsda 16, Lexception0<br>
## BB#0:<br>
pushq %rbp<br>
Ltmp3:<br>
.cfi_def_cfa_offset 16<br>
Ltmp4:<br>
.cfi_offset %rbp, -16<br>
movq %rsp, %rbp<br>
Ltmp5:<br>
.cfi_def_cfa_register %rbp<br>
pushq %r14<br>
pushq %rbx<br>
Ltmp6:<br>
.cfi_offset %rbx, -32<br>
Ltmp7:<br>
.cfi_offset %r14, -24<br>
movq %rsi, %r14<br>
movq %rdi, %rbx<br>
testb $1, (%rbx)<br>
jne LBB0_1<br>
## BB#2:<br>
movw $0, (%rbx)<br>
jmp LBB0_3<br>
LBB0_1:<br>
movq 16(%rbx), %rax<br>
movb $0, (%rax)<br>
movq $0, 8(%rbx)<br>
LBB0_3: ## %_ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEE5clearEv.exit.i.i<br>
Ltmp0:<br>
xorl %esi, %esi<br>
movq %rbx, %rdi<br>
callq __ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEE7reserveEm<br>
Ltmp1:<br>
## BB#4: ## %_ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_.exit<br>
movq 16(%r14), %rax<br>
movq %rax, 16(%rbx)<br>
movq (%r14), %rax<br>
movq 8(%r14), %rcx<br>
movq %rcx, 8(%rbx)<br>
movq %rax, (%rbx)<br>
movq $0, 16(%r14)<br>
movq $0, 8(%r14)<br>
movq $0, (%r14)<br>
popq %rbx<br>
popq %r14<br>
popq %rbp<br>
retq<br>
LBB0_5:<br>
Ltmp2:<br>
movq %rax, %rdi<br>
callq ___clang_call_terminate<br>
Lfunc_end0:<br>
.cfi_endproc<br>
.section __TEXT,__gcc_except_tab<br>
.align 2<br>
GCC_except_table0:<br>
Lexception0:<br>
.byte 255 ## @LPStart Encoding = omit<br>
.byte 155 ## @TType Encoding = indirect pcrel sdata4<br>
.byte 21 ## @TType base offset<br>
.byte 3 ## Call site Encoding = udata4<br>
.byte 13 ## Call site table length<br>
Lset0 = Ltmp0-Lfunc_begin0 ## >> Call Site 1 <<<br>
.long Lset0<br>
Lset1 = Ltmp1-Ltmp0 ## Call between Ltmp0 and Ltmp1<br>
.long Lset1<br>
Lset2 = Ltmp2-Lfunc_begin0 ## jumps to Ltmp2<br>
.long Lset2<br>
.byte 1 ## On action: 1<br>
.byte 1 ## >> Action Record 1 <<<br>
## Catch TypeInfo 1<br>
.byte 0 ## No further actions<br>
## >> Catch TypeInfos <<<br>
.long 0 ## TypeInfo 1<br>
.align 2<br>
<br>
.section __TEXT,__textcoal_nt,coalesced,pure_instructions<br>
.private_extern ___clang_call_terminate<br>
.globl ___clang_call_terminate<br>
.weak_def_can_be_hidden ___clang_call_terminate<br>
.align 4, 0x90<br>
___clang_call_terminate: ## @__clang_call_terminate<br>
## BB#0:<br>
pushq %rbp<br>
movq %rsp, %rbp<br>
callq ___cxa_begin_catch<br>
callq __ZSt9terminatev<br>
<br>
<br>
.subsections_via_symbols<br>
<br>
which is a mess of code much of which *never* gets executed. There’s even an exception handling table complete with call to terminate (awful). This can be vastly improved with just a small change:<br>
<br>
index 6ed4e74..0d07cf7 100644<br>
--- a/include/string<br>
+++ b/include/string<br>
@@ -2453,8 +2453,8 @@ basic_string<_CharT, _Traits, _Allocator>::__move_assign(basic_string& __str, tr<br>
_NOEXCEPT_(is_nothrow_move_assignable<allocator_type>::value)<br>
#endif<br>
{<br>
- clear();<br>
- shrink_to_fit();<br>
+ if (__is_long())<br>
+ __alloc_traits::deallocate(__alloc(), __get_long_pointer(), __get_long_cap());<br>
__r_.first() = __str.__r_.first();<br>
__move_assign_alloc(__str);<br>
__str.__zero();<br>
<br>
This is equivalent functionality as what is there today, but compiles down to this instead:<br>
<br>
.section __TEXT,__text,regular,pure_instructions<br>
.macosx_version_min 10, 10<br>
.globl __Z4testRNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEOS5_<br>
.align 4, 0x90<br>
__Z4testRNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEOS5_: ## @_Z4testRNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEOS5_<br>
.cfi_startproc<br>
## BB#0:<br>
pushq %rbp<br>
Ltmp0:<br>
.cfi_def_cfa_offset 16<br>
Ltmp1:<br>
.cfi_offset %rbp, -16<br>
movq %rsp, %rbp<br>
Ltmp2:<br>
.cfi_def_cfa_register %rbp<br>
pushq %r14<br>
pushq %rbx<br>
Ltmp3:<br>
.cfi_offset %rbx, -32<br>
Ltmp4:<br>
.cfi_offset %r14, -24<br>
movq %rsi, %rbx<br>
movq %rdi, %r14<br>
testb $1, (%r14)<br>
je LBB0_2<br>
## BB#1:<br>
movq 16(%r14), %rdi<br>
callq __ZdlPv<br>
LBB0_2: ## %_ZNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEaSEOS5_.exit<br>
movq 16(%rbx), %rax<br>
movq %rax, 16(%r14)<br>
movq (%rbx), %rax<br>
movq 8(%rbx), %rcx<br>
movq %rcx, 8(%r14)<br>
movq %rax, (%r14)<br>
movq $0, 16(%rbx)<br>
movq $0, 8(%rbx)<br>
movq $0, (%rbx)<br>
popq %rbx<br>
popq %r14<br>
popq %rbp<br>
retq<br>
.cfi_endproc<br>
<br>
<br>
.subsections_via_symbols<br>
<br>
Not only is this smaller code, it completely eliminates the exception handling table, and completely eliminates a mandatory call into reserve(), which itself is going to have several more branches in it.<br>
<br>
Howard<br>
<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div><br></div>