Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Missed optimization: useless write of zero to memory #31494

Open Quuxplusone opened 7 years ago

Quuxplusone commented 7 years ago
Bugzilla Link PR32522
Status NEW
Importance P enhancement
Reported by Antony Polukhin (antoshkka@gmail.com)
Reported on 2017-04-04 12:43:38 -0700
Last modified on 2017-05-24 03:35:19 -0700
Version unspecified
Hardware PC Linux
CC antoshkka@gmail.com, efriedma@quicinc.com, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
The following code:

///////////////
#include <new>

template <class T>
struct shared_ptr_like {
    T* ptr;
    shared_ptr_like(shared_ptr_like&& v) noexcept : ptr(v.ptr) { v.ptr = 0; }
    ~shared_ptr_like() { if (ptr) { delete ptr; } }
};

typedef shared_ptr_like<int> test_type;
void relocate(test_type* new_buffer, test_type* old_buffer) {
        new (new_buffer) test_type{ static_cast<test_type&&>(*old_buffer) };
        old_buffer->~test_type();
}
///////////////

Generates suboptimal assembly:

relocate(shared_ptr_like<int>*, shared_ptr_like<int>*):    #
@relocate(shared_ptr_like<int>*, shared_ptr_like<int>*)
        mov     rax, qword ptr [rsi]
        mov     qword ptr [rdi], rax
        mov     qword ptr [rsi], 0 # Remove this
        ret

For that example GCC 7.0.1 20170330 with flags -std=c++17 -O2 generates
following assembly

relocate(shared_ptr_like<int>*, shared_ptr_like<int>*):
        mov     rax, QWORD PTR [rsi]
        mov     QWORD PTR [rdi], rax
        ret

This is allowed by C++ standard because
* [class.dtor] specifies that "Once a destructor is invoked for an object, the
object no longer exists<...>"
* all the accesses to stored values require an object (if object does not exist
- accessing value is UB) [basic.lval] "If a program attempts to access the
stored value of an object through a glvalue <...>"

Described optimization is essential because code that moves an object to new
location and then destroys it often used in loops (for example
std::vector::reserve does that).

Checked on CLANG 4.0 (tags/RELEASE_400/final 297782) with flags -std=c++1z -O2
Quuxplusone commented 7 years ago

clang currently doesn't currently emit any information about destructor calls into the IR that would allow removing the store.