[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335 --- Comment #10 from Jonathan Wakely --- Ah, now I understand what you've been saying about the postcondition. Yes, but the compiler doesn't know the postcondition, it's just words in the standard, so not visible to the optimization passes. It might be possible to add some optimization hints to the std::unique_ptr implementation to make the postcondition visible to the optimizer.
[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335
--- Comment #9 from Federico Kircheis ---
Great, thank you for the clarifications, your redacted example makes now sense.
> https://godbolt.org/z/WGPTesEb3 shows that bar3 is not the same as bar1,
> because it runs an additional destructor if operator delete() plays silly
> games.
And this is what I meant before that such program is not supported by the
standard
void bar3(std::unique_ptr& ps1, std::unique_ptr& ps2){
ps1.reset();
ps1.reset();
ps1 = std::move(ps2);
}
---
is equivalent to
void bar3(std::unique_ptr& ps1, std::unique_ptr& ps2){
ps1.reset();
assert(ps1 == nullptr);
ps1.reset();
assert(ps1 == nullptr);
ps1 = std::move(ps2);
}
which is not.
And the same holds if "if (!global) global.reset(new s);" is moved into "~s()",
and removed from "operator delete".
If the users changes delete in such a way, at this point we have UB, as the
postcondition of unique_ptr::reset does not hold (the example given by Andrew)
Yes, the second ps1.reset(); is trivial to remove.
[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335
--- Comment #8 from Jonathan Wakely ---
(In reply to Federico Kircheis from comment #7)
> > Demo: https://godbolt.org/z/PWd96j4fz
>
> Thank you for the example, but honestly, I think I am still missing
> something.
>
> If you modify the main this way
>
>
> int main()
> {
> global = nullptr;
> std::unique_ptr local1(new s);
> bar1(global, local1);
> global = nullptr;
> std::unique_ptr local2(new s);
> bar1(global, local2);
> }
>
>
>
> or this way
>
>
>
> int main()
> {
> global = nullptr;
> std::unique_ptr local1(new s);
> bar2(global, local1);
> global = nullptr;
> std::unique_ptr local2(new s);
> bar2(global, local2);
> }
>
>
> the output is the same.
> First time it prints 0, the second time a non-0 address.
You're right, my example failed to show the difference I was trying to show,
which is that the value of ps1.get() observed by the destructor is different.
To show that, global.get() must be non-null initially, but it was always nul in
my example (the addresses being printed out where when the destructors ran in
main, not in bar1 and bar2).
Try this one: https://godbolt.org/z/KEbnPxEdq
This shows that in bar1 the value of ps1.get() is null when the destructor
runs. In bar2 the value of ps2.get() is not null when the destructor runs.
>
> > But a much simpler example where the difference is observable is when the
> > two arguments to bar1 and bar2 are the same object:
> > https://godbolt.org/z/z8fsTan8K
>
> And this is absolutely correct, did not think of it.
>
> > I'm not sure what you mean here. The postcondition holds unless the deleter
> > *also* destroys the unique_ptr itself.
>
> As written, I was not sure if this plays a role; this is what the standard
> says (and I#ve misunderstood the example of Andrew).
> A similar effect can be observed in the following example
>
>
> #include
>
> using s = int; // for simplicity
>
> void bar1(std::unique_ptr& ps1, std::unique_ptr& ps2){
> ps1.reset();
> ps1 = std::move(ps2);
> }
> void bar2(std::unique_ptr& ps1, std::unique_ptr& ps2){
> ps1 = std::move(ps2);
> }
>
> void bar3(std::unique_ptr& ps1, std::unique_ptr& ps2){
> ps1.reset();
> ps1.reset();
> ps1 = std::move(ps2);
> }
>
>
> While you are right that bar1 and bar2 have different behavior if they point
> to the same object, bar1 and bar3 are fundamentally the same function, even
> in case of aliasing.
> Unless the post-condition of reset is not hold; otherwise the second
> "ps1.reset()" could be completely removed, as ps1 is nullptr.
The postcondition has to hold, by definition, that's what it means. It only
doesn't hold if the unique_ptr object itself ceases to exist, and that doesn't
happen with std::default_delete. It's irrelevant whether it happens with some
other deleters, because the code you're compiling doesn't use those other
deleters (if it did, it would be compiled differently).
This example is more interesting, bar1 does:
testr12, r12
je .L11
mov rdi, r12
calls::~s() [complete object destructor]
mov rdi, r12
mov esi, 1
calloperator delete(void*, unsigned long)
But bar3 has extra instructions:
testr12, r12
je .L11
mov rdi, r12
calls::~s() [complete object destructor]
mov rdi, r12
mov esi, 1
calloperator delete(void*, unsigned long)
mov r12, QWORD PTR [rbx]
mov QWORD PTR [rbx], 0
testr12, r12
je .L11
mov rdi, r12
calls::~s() [complete object destructor]
mov esi, 1
mov rdi, r12
calloperator delete(void*, unsigned long)
I think the reason the redundant ps1.reset() isn't removed is that the compiler
doesn't know what operator delete(void*, size_t) does. In theory, the user
could replace that function with one which modifies a global unique_ptr, and
ps1 could alias that global.
https://godbolt.org/z/WGPTesEb3 shows that bar3 is not the same as bar1,
because it runs an additional destructor if operator delete() plays silly
games.
If we had PR 110137 then I think the redundant ps1.reset() would be optimized
away. Without it, the compiler assumes operator delete() might be silly.
Luckily, that bar3 code seems unlikely to be written. The second ps1.reset()
can obviously be removed by the programmer.
[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335
--- Comment #7 from Federico Kircheis ---
Thank you Jonathan, I've then misunderstood the example of Andrew, I thought he
was trying to create a scenario where after reset the pointer is not nullptr.
> Demo: https://godbolt.org/z/PWd96j4fz
Thank you for the example, but honestly, I think I am still missing something.
If you modify the main this way
int main()
{
global = nullptr;
std::unique_ptr local1(new s);
bar1(global, local1);
global = nullptr;
std::unique_ptr local2(new s);
bar1(global, local2);
}
or this way
int main()
{
global = nullptr;
std::unique_ptr local1(new s);
bar2(global, local1);
global = nullptr;
std::unique_ptr local2(new s);
bar2(global, local2);
}
the output is the same.
First time it prints 0, the second time a non-0 address.
> But a much simpler example where the difference is observable is when the two
> arguments to bar1 and bar2 are the same object:
> https://godbolt.org/z/z8fsTan8K
And this is absolutely correct, did not think of it.
> I'm not sure what you mean here. The postcondition holds unless the deleter
> *also* destroys the unique_ptr itself.
As written, I was not sure if this plays a role; this is what the standard says
(and I#ve misunderstood the example of Andrew).
A similar effect can be observed in the following example
#include
using s = int; // for simplicity
void bar1(std::unique_ptr& ps1, std::unique_ptr& ps2){
ps1.reset();
ps1 = std::move(ps2);
}
void bar2(std::unique_ptr& ps1, std::unique_ptr& ps2){
ps1 = std::move(ps2);
}
void bar3(std::unique_ptr& ps1, std::unique_ptr& ps2){
ps1.reset();
ps1.reset();
ps1 = std::move(ps2);
}
While you are right that bar1 and bar2 have different behavior if they point to
the same object, bar1 and bar3 are fundamentally the same function, even in
case of aliasing.
Unless the post-condition of reset is not hold; otherwise the second
"ps1.reset()" could be completely removed, as ps1 is nullptr.
[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335 Jonathan Wakely changed: What|Removed |Added Resolution|--- |INVALID Status|UNCONFIRMED |RESOLVED --- Comment #6 from Jonathan Wakely --- (In reply to Federico Kircheis from comment #5) > I do not think that the case you have in mind is supported by the standard, It is. The standard is very clear about exactly which operations happen, and in which order. Code can rely on that behaviour, because it's guaranteed by the standard. Implementations are not allowed to deviate from that. In your 'bar1' function when the ~s destructor runs it is guaranteed that it will observe ps1.get() == nullptr, because that's the first thing that ps1.reset() does. In your 'bar2' function when the ~s destructor runs it's guaranteed that it will observe ps1.get() == the original value of ps2, because that value gets set before invoking the deleter. Demo: https://godbolt.org/z/PWd96j4fz That is clearly an observable difference, which is required by the standard, and so your bar1 and bar2 are clearly not equivalent. But a much simpler example where the difference is observable is when the two arguments to bar1 and bar2 are the same object: https://godbolt.org/z/z8fsTan8K For bar1 you destroy any pointed-to object, then move-assign an empty pointer to itself, which does nothing. So bar1(p, p,) is equivalent to p.reset(); For bar2 you release the pointer, then reset it back again. So bar2(p, p) is a no-op. The functions are not clearly equivalent. > but the postcondition does not hold with some deleters, so not sure if that > could be relevant here I'm not sure what you mean here. The postcondition holds unless the deleter *also* destroys the unique_ptr itself.
[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335
--- Comment #5 from Federico Kircheis ---
Ok, the described case would be something like
std::unique_ptr t;
__thread bool tt;
inline s::~s()
{
if (tt)
return;
tt = true;
t.reset(new s);
tt = false;
}
std::unique_ptr t2;
bar(t, t2);
The postcondition of p.reset() is that p == nullptr.
void reset(pointer p = pointer()) noexcept;
Preconditions: The expression get_deleter()(get()) is well-formed, has
well-defined behavior, and does not throw exceptions.
Effects: Assigns p to the stored pointer, and then if and only if the old value
of the stored pointer, old_p, was not equal to nullptr, calls
get_deleter()(old_p). [Note: The order of these operations is significant
because the call to get_deleter() may destroy *this. — end note]
Postconditions: get() == p. [Note: The postcondition does not hold if the call
to get_deleter() destroys *this since this->get() is no longer a valid
expression. — end note]
I do not think that the case you have in mind is supported by the standard, but
the postcondition does not hold with some deleters, so not sure if that could
be relevant here
[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335
--- Comment #4 from Andrew Pinski ---
(In reply to Federico Kircheis from comment #2)
> > Well s::~s could touch the reference std::unique_ptr (ps1).
>
> In both cases, s::~s is called only once.
>
> Also during the move-assignment no user-provided-code is involved (except
> the destruction of ps1, and even case of a user-provided deleter, it is only
> called if the pointer is not nullptr.
NO. NO.
std::unique_ptr t;
__thread bool tt;
inline s::~s()
{
if (tt)
return;
tt = true;
t.reset(new s);
tt = false;
}
Is still a valid ~s as far as I can tell.
Yes it is odd but so what it is still valid.
Now will anyone write code like this, doubt it.
Also note clang does not optimize this with either libstdc++ or libc++.
[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335 --- Comment #3 from Federico Kircheis --- Or maybe I've misunderstood your comment. Do you have a specific scenario in mind where the two snippets would exhibit different behaviors?
[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335 --- Comment #2 from Federico Kircheis --- > Well s::~s could touch the reference std::unique_ptr (ps1). In both cases, s::~s is called only once. Also during the move-assignment no user-provided-code is involved (except the destruction of ps1, and even case of a user-provided deleter, it is only called if the pointer is not nullptr.
[Bug c++/112335] missed optimization on reset and assign unique_ptr
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112335 --- Comment #1 from Andrew Pinski --- __old_p_6 = MEM[(struct s * &)ps1_2(D)]; MEM[(struct s * &)ps1_2(D)] = 0B; if (__old_p_6 != 0B) goto ; [53.47%] else goto ; [46.53%] [local count: 574129752]: s::~s (__old_p_6); operator delete (__old_p_6, 1); [local count: 1073741824]: __p_4 = MEM[(struct s * &)ps2_3(D)]; MEM[(struct s * &)ps2_3(D)] = 0B; __old_p_5 = MEM[(struct s * &)ps1_2(D)]; Well s::~s could touch the reference std::unique_ptr (ps1). So this could in theory be an invalid optimization. I am not sure if that is the only issue here though.
