public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/102490] New: Erroneous optimization of default constexpr operator== of struct with bitfields
@ 2021-09-26 11:31 luc.briand35 at gmail dot com
2021-09-27 10:16 ` [Bug c++/102490] " jakub at gcc dot gnu.org
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: luc.briand35 at gmail dot com @ 2021-09-26 11:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102490
Bug ID: 102490
Summary: Erroneous optimization of default constexpr operator==
of struct with bitfields
Product: gcc
Version: 10.1.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: luc.briand35 at gmail dot com
Target Milestone: ---
Hello,
For the following code, gcc version 10 and up wrongly optimizes the default
operator==().
This occurs for O1 and up.
Removing the 'constexpr' qualifier fixes everything.
The size of the bitfields doesn't matter.
No warnings are appear with "-Wall -Wextra".
Godbolt link: https://gcc.godbolt.org/z/j4fG3sKze
struct A
{
unsigned char foo : 1;
unsigned char bar : 1;
constexpr bool operator==(const A&) const = default;
};
int main()
{
A a{}, b{};
a.bar = 0b1;
return a == b;
}
With the options "-std=c++2a -O1", the assembly generated is simply:
main:
mov eax, 1
ret
In this similar example, we can see that the generated assembly for the
equality operator ignores the 'bar' bitfield (Godbolt link:
https://gcc.godbolt.org/z/3K75xx1on) :
struct A
{
unsigned char foo : 3;
unsigned char bar : 1;
constexpr bool operator==(const A&) const = default;
};
void change(A& a);
int main()
{
A a{}, b{};
change(a);
return a == b;
}
The assembly for gcc version 10.X and 11.X is a bit different, but have the
same problem.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug c++/102490] Erroneous optimization of default constexpr operator== of struct with bitfields
2021-09-26 11:31 [Bug c++/102490] New: Erroneous optimization of default constexpr operator== of struct with bitfields luc.briand35 at gmail dot com
@ 2021-09-27 10:16 ` jakub at gcc dot gnu.org
2021-09-27 11:33 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-09-27 10:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102490
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
CC| |jakub at gcc dot gnu.org
Last reconfirmed| |2021-09-27
Status|UNCONFIRMED |NEW
--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Before r10-4397-gb7689b962dd6536bbb2567bfdec52e35109af7ab , this has been
rejected with:
pr102490.C:5:48: error: ‘constexpr bool A::operator==(const A&) const’ cannot
be defaulted
5 | constexpr bool operator== (const A&) const = default;
| ^~~~~~~
pr102490.C:5:18: warning: inline function ‘constexpr bool A::operator==(const
A&) const’ used but never defined
5 | constexpr bool operator== (const A&) const = default;
| ^~~~~~~~
but since then it emits:
pr102490.C: In member function ‘constexpr bool A::operator==(const A&) const’:
pr102490.C:5:18: error: type mismatch in ‘component_ref’
5 | constexpr bool operator== (const A&) const = default;
| ^~~~~~~~
unsigned char
<unnamed-unsigned:1>
_1 = this->foo;
pr102490.C:5:18: error: type mismatch in ‘component_ref’
unsigned char
<unnamed-unsigned:1>
_2 = D.2080->foo;
pr102490.C:5:18: error: type mismatch in ‘component_ref’
unsigned char
<unnamed-unsigned:1>
_3 = this->bar;
pr102490.C:5:18: error: type mismatch in ‘component_ref’
unsigned char
<unnamed-unsigned:1>
_4 = D.2080->bar;
pr102490.C:5:18: internal compiler error: ‘verify_gimple’ failed
at -O0 or miscompiles it at -O1.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug c++/102490] Erroneous optimization of default constexpr operator== of struct with bitfields
2021-09-26 11:31 [Bug c++/102490] New: Erroneous optimization of default constexpr operator== of struct with bitfields luc.briand35 at gmail dot com
2021-09-27 10:16 ` [Bug c++/102490] " jakub at gcc dot gnu.org
@ 2021-09-27 11:33 ` jakub at gcc dot gnu.org
2021-09-27 11:37 ` pinskia at gcc dot gnu.org
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-09-27 11:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102490
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jason at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The bug is that the operator== is synthetized before the bitfields are adjusted
to the middle-end way, but assumes they are already adjusted that way.
In particular, the synthetization is done from:
#8 0x0000000000c8fe64 in build_comparison_op (fndecl=<function_decl
0x7fffea1a5500 operator==>, complain=0) at ../../gcc/cp/method.c:1454
#9 0x0000000000c91895 in synthesize_method (fndecl=<function_decl
0x7fffea1a5500 operator==>) at ../../gcc/cp/method.c:1779
#10 0x0000000000c98e1a in defaulted_late_check (fn=<function_decl
0x7fffea1a5500 operator==>) at ../../gcc/cp/method.c:3168
#11 0x0000000000b10439 in check_bases_and_members (t=<record_type
0x7fffea1a1b28 A>) at ../../gcc/cp/class.c:6136
#12 0x0000000000b161c8 in finish_struct_1 (t=<record_type 0x7fffea1a1b28 A>) at
../../gcc/cp/class.c:7372
#13 0x0000000000b17d3d in finish_struct (t=<record_type 0x7fffea1a1b28 A>,
attributes=<tree 0x0>) at ../../gcc/cp/class.c:7668
but the bitfields are adjusted in
#0 layout_class_type (t=<record_type 0x7fffea1a1b28 A>,
virtuals_p=0x7fffffffcef0) at ../../gcc/cp/class.c:6711
#1 0x0000000000b162ae in finish_struct_1 (t=<record_type 0x7fffea1a1b28 A>) at
../../gcc/cp/class.c:7397
#2 0x0000000000b17d3d in finish_struct (t=<record_type 0x7fffea1a1b28 A>,
attributes=<tree 0x0>) at ../../gcc/cp/class.c:7668
So, we emit e.g. the COMPONENT_REFs with the underlying bitfield type, but then
layout_class_type changes the type of the FIELD_DECLs to something else and
we'd
instead want to either compare in the actual bitfield types, or on NOP_EXPRs
from the COMPONENT_REFs to the underlying type.
So, either build_comparison_op should check for DECL_C_BIT_FIELD fields whether
TREE_TYPE of the field has the DECL_SIZE precision or not (but we'd also need
to handle there e.g. the
/* If this field is a bit-field whose width is greater than its
type, then there are some special rules for allocating
it. */
if (DECL_C_BIT_FIELD (field)
&& tree_int_cst_lt (TYPE_SIZE (type), DECL_SIZE (field)))
stuff from layout_class_type (as well as the:
/* The middle end uses the type of expressions to determine the
possible range of expression values. In order to optimize
"x.i > 7" to "false" for a 2-bit bitfield "i", the middle end
must be made aware of the width of "i", via its type.
Because C++ does not have integer types of arbitrary width,
we must (for the purposes of the front end) convert from the
type assigned here to the declared type of the bitfield
whenever a bitfield expression is used as an rvalue.
Similarly, when assigning a value to a bitfield, the value
must be converted to the type given the bitfield here. */
if (DECL_C_BIT_FIELD (field))
), or we need to somehow make sure that the == and <=> methods are synthetized
only after that happens. Jason, thoughts on that?
Also, wonder about :0 bitfields in classes that have defaulted comparisons...
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug c++/102490] Erroneous optimization of default constexpr operator== of struct with bitfields
2021-09-26 11:31 [Bug c++/102490] New: Erroneous optimization of default constexpr operator== of struct with bitfields luc.briand35 at gmail dot com
2021-09-27 10:16 ` [Bug c++/102490] " jakub at gcc dot gnu.org
2021-09-27 11:33 ` jakub at gcc dot gnu.org
@ 2021-09-27 11:37 ` pinskia at gcc dot gnu.org
2021-09-27 12:38 ` jakub at gcc dot gnu.org
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-09-27 11:37 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102490
--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 102488 has been marked as a duplicate of this bug. ***
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug c++/102490] Erroneous optimization of default constexpr operator== of struct with bitfields
2021-09-26 11:31 [Bug c++/102490] New: Erroneous optimization of default constexpr operator== of struct with bitfields luc.briand35 at gmail dot com
` (2 preceding siblings ...)
2021-09-27 11:37 ` pinskia at gcc dot gnu.org
@ 2021-09-27 12:38 ` jakub at gcc dot gnu.org
2021-10-05 17:15 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-09-27 12:38 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102490
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51511
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51511&action=edit
gcc12-pr102490.patch
Untested fix.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug c++/102490] Erroneous optimization of default constexpr operator== of struct with bitfields
2021-09-26 11:31 [Bug c++/102490] New: Erroneous optimization of default constexpr operator== of struct with bitfields luc.briand35 at gmail dot com
` (3 preceding siblings ...)
2021-09-27 12:38 ` jakub at gcc dot gnu.org
@ 2021-10-05 17:15 ` jakub at gcc dot gnu.org
2021-10-06 2:41 ` cvs-commit at gcc dot gnu.org
2022-11-03 21:18 ` ppalka at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-10-05 17:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102490
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|jakub at gcc dot gnu.org |jason at gcc dot gnu.org
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Latest patch version
https://gcc.gnu.org/pipermail/gcc-patches/2021-October/580706.html
Jason was kind enough to take it over as more work needs to be done, e.g. to
make the synthesization work on binfos rather than base FIELD_DECLs that might
not exist yet.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug c++/102490] Erroneous optimization of default constexpr operator== of struct with bitfields
2021-09-26 11:31 [Bug c++/102490] New: Erroneous optimization of default constexpr operator== of struct with bitfields luc.briand35 at gmail dot com
` (4 preceding siblings ...)
2021-10-05 17:15 ` jakub at gcc dot gnu.org
@ 2021-10-06 2:41 ` cvs-commit at gcc dot gnu.org
2022-11-03 21:18 ` ppalka at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-10-06 2:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102490
--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:
https://gcc.gnu.org/g:09d886e671f2230acca082e6579a69b8df8fb202
commit r12-4202-g09d886e671f2230acca082e6579a69b8df8fb202
Author: Jakub Jelinek <jakub@redhat.com>
Date: Fri Oct 1 17:07:17 2021 +0200
c++: defaulted <=> with bitfields [PR102490]
The testcases in the patch are either miscompiled or ICE with checking,
because the defaulted operator== is synthesized too early (but only if
constexpr), when the corresponding class type is still incomplete type.
The
problem is that at that point the bitfield FIELD_DECLs still have as
TREE_TYPE their underlying type rather than integral type with their
precision and when layout_class_type is called for the class soon after
that, it changes those types but the COMPONENT_REFs type stay the way that
they were during the operator== synthesize_method type and the middle-end
is
then upset by the mismatch of types. As what exact type will be given
isn't
just a one liner but quite long code especially for over-sized bitfields, I
think it is best to just not synthesize the comparison operators so early
and call defaulted_late_check for them once again as soon as the class is
complete.
This is also a problem for virtual operator<=>, where we need to compare
the
noexcept-specifier to validate the override before the class is complete.
Rather than try to defer that comparison, maybe_instantiate_noexcept now
calls maybe_synthesize_method, which calls build_comparison_op in
non-defining mode if the class isn't complete yet. In that situation we
also might not have base fields yet, so we look in the binfo for the bases.
Co-authored-by: Jason Merrill <jason@redhat.com>
2021-10-01 Jakub Jelinek <jakub@redhat.com>
PR c++/98712
PR c++/102490
* cp-tree.h (maybe_synthesize_method): Declare.
* method.c (genericize_spaceship): Use
LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED instead of
LOOKUP_NORMAL for flags.
(comp_info): Remove defining member. Add complain, code, retcat.
(comp_info::comp_info): Adjust.
(do_one_comp): Split out from build_comparison_op. Use
LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED instead of
LOOKUP_NORMAL for flags.
(build_comparison_op): Add defining argument. Adjust comp_info
construction. Use defining instead of info.defining. Assert that
if defining, ctype is a complete type. Walk base binfos.
(synthesize_method, maybe_explain_implicit_delete,
explain_implicit_non_constexpr): Adjust build_comparison_op
callers.
(maybe_synthesize_method): New function.
* class.c (check_bases_and_members): Don't call
defaulted_late_check
for sfk_comparison.
(finish_struct_1): Call it here instead after class has been
completed.
* pt.c (maybe_instantiate_noexcept): Call maybe_synthesize_method
instead of synthesize_method.
* g++.dg/cpp2a/spaceship-synth8.C (std::strong_ordering): Provide
more complete definition.
(std::strong_ordering::less, std::strong_ordering::equal,
std::strong_ordering::greater): Define.
* g++.dg/cpp2a/spaceship-synth12.C: New test.
* g++.dg/cpp2a/spaceship-synth13.C: New test.
* g++.dg/cpp2a/spaceship-synth14.C: New test.
* g++.dg/cpp2a/spaceship-eq11.C: New test.
* g++.dg/cpp2a/spaceship-eq12.C: New test.
* g++.dg/cpp2a/spaceship-eq13.C: New test.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug c++/102490] Erroneous optimization of default constexpr operator== of struct with bitfields
2021-09-26 11:31 [Bug c++/102490] New: Erroneous optimization of default constexpr operator== of struct with bitfields luc.briand35 at gmail dot com
` (5 preceding siblings ...)
2021-10-06 2:41 ` cvs-commit at gcc dot gnu.org
@ 2022-11-03 21:18 ` ppalka at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: ppalka at gcc dot gnu.org @ 2022-11-03 21:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102490
Patrick Palka <ppalka at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
CC| |ppalka at gcc dot gnu.org
Resolution|--- |FIXED
Target Milestone|--- |12.0
--- Comment #7 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Thus fixed since GCC 12
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-03 21:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-26 11:31 [Bug c++/102490] New: Erroneous optimization of default constexpr operator== of struct with bitfields luc.briand35 at gmail dot com
2021-09-27 10:16 ` [Bug c++/102490] " jakub at gcc dot gnu.org
2021-09-27 11:33 ` jakub at gcc dot gnu.org
2021-09-27 11:37 ` pinskia at gcc dot gnu.org
2021-09-27 12:38 ` jakub at gcc dot gnu.org
2021-10-05 17:15 ` jakub at gcc dot gnu.org
2021-10-06 2:41 ` cvs-commit at gcc dot gnu.org
2022-11-03 21:18 ` ppalka 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).