public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/111388] New: std:.get_if variant, unnecessary branch when outside of if statement
@ 2023-09-12 11:13 federico at kircheis dot it
2023-09-12 14:55 ` [Bug tree-optimization/111388] " pinskia at gcc dot gnu.org
2023-09-12 14:58 ` pinskia at gcc dot gnu.org
0 siblings, 2 replies; 3+ messages in thread
From: federico at kircheis dot it @ 2023-09-12 11:13 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111388
Bug ID: 111388
Summary: std:.get_if variant, unnecessary branch when outside
of if statement
Product: gcc
Version: 13.2.1
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: federico at kircheis dot it
Target Milestone: ---
Example of code (https://godbolt.org/z/M1bWf5sz3)
----
#include <variant>
struct interface{
virtual ~interface()= default;
virtual int foo() const = 0;
};
struct a : interface{
int foo() const override;
};
struct b : interface{
int foo() const override;
};
class a_or_b{
std::variant<a,b> ab;
public:
a_or_b() = delete;
a_or_b(a _) : ab(_){}
a_or_b(b _) : ab(_){}
interface* operator->() noexcept {
if (interface* ptr = std::get_if<0>(&ab); ptr) {
return ptr;
}
#if 0
else if (interface* ptr = std::get_if<1>(&ab); ptr) {
return ptr;
}
#else
return std::get_if<1>(&ab);
#endif
}
};
int bar3(a_or_b& ab){
return ab->foo()+1;
}
----
With `#if 1`, the generated code looks like
----
bar3(a_or_b&):
sub rsp, 8
mov rax, QWORD PTR [rdi]
call [QWORD PTR [rax+16]]
add rsp, 8
add eax, 1
ret
----
while with `#if 0`, the assembly looks like
----
bar3(a_or_b&):
cmp BYTE PTR [rdi+8], 0
jne .L2
sub rsp, 8
mov rax, QWORD PTR [rdi]
call [QWORD PTR [rax+16]]
add rsp, 8
add eax, 1
ret
bar3(a_or_b&) [clone .cold]:
.L2:
mov rax, QWORD PTR ds:0
ud2
----
If I'm not mistake, with `return std::get_if<1>(&ab);` the compiler verifies if
the return of get_if is nullptr, and if it is, then sets the return value to
nullptr, which is unnecessary.
With `if 1`, the result is passed as is.
AFAIK the generated assembly is functionally equivalant, but the "more safe"(1)
version less optimal
1) more safe as in "there is no UB if the class changes and the variant could
be empty or hold another type".
NOTE: replacing "std::get_if<0>"/"std::get_if<1>" with
"std::get_if<a>/std::get_if<b>" does not make a relevante difference, the
generated code is the same for both "if 0" and "if 1".
For what is worth, clang generates the same code for "if 0" and "if 1".
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Bug tree-optimization/111388] std:.get_if variant, unnecessary branch when outside of if statement
2023-09-12 11:13 [Bug c++/111388] New: std:.get_if variant, unnecessary branch when outside of if statement federico at kircheis dot it
@ 2023-09-12 14:55 ` pinskia at gcc dot gnu.org
2023-09-12 14:58 ` pinskia at gcc dot gnu.org
1 sibling, 0 replies; 3+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-09-12 14:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111388
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note variant can still hold neither ...
You might need to add a check for valueless_by_exception here.
But variant::index could be improved to say the only values that are valid is
[0,N],[-1]. and that will remove the check.
Note clang even produces the similar code even with libc++:
xorl %eax, %eax
cmpl $2, 8(%rdi)
cmovaeq %rax, %rdi
Just GCC isolates the null pointer access.
^ permalink raw reply [flat|nested] 3+ messages in thread
* [Bug tree-optimization/111388] std:.get_if variant, unnecessary branch when outside of if statement
2023-09-12 11:13 [Bug c++/111388] New: std:.get_if variant, unnecessary branch when outside of if statement federico at kircheis dot it
2023-09-12 14:55 ` [Bug tree-optimization/111388] " pinskia at gcc dot gnu.org
@ 2023-09-12 14:58 ` pinskia at gcc dot gnu.org
1 sibling, 0 replies; 3+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-09-12 14:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111388
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Adding:
```
if (ab.index()>=2)
__builtin_unreachable();
```
to operator->
Also fixes the issue.
C++23 would be:
[[assume(ab.index()<2)]];
(Except that does not optimize currently).
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-12 14:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-12 11:13 [Bug c++/111388] New: std:.get_if variant, unnecessary branch when outside of if statement federico at kircheis dot it
2023-09-12 14:55 ` [Bug tree-optimization/111388] " pinskia at gcc dot gnu.org
2023-09-12 14:58 ` pinskia at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).