public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels
@ 2020-07-09 14:01 tobi at gcc dot gnu.org
2020-07-10 7:19 ` [Bug tree-optimization/96135] " rguenth at gcc dot gnu.org
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: tobi at gcc dot gnu.org @ 2020-07-09 14:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
Bug ID: 96135
Summary: [9/10/11 regression] bswap not detected by bswap pass,
unexpected results between optimization levels
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: tobi at gcc dot gnu.org
Target Milestone: ---
This is an odd one, and it seems different from the other bswap bugs that I
could find in bugzilla. This is on x64.
Compiler Explorer link is here: https://godbolt.org/z/arTf5T
Full source code:
==============================================================
constexpr long long bswap64(long long in) // unsigned long long behaves the
same
{
union {
long long v;
char c[8];
} u{in};
union {
char c[8];
long long v;
} v{ u.c[7], u.c[6], u.c[5], u.c[4], u.c[3], u.c[2], u.c[1], u.c[0]};
return v.v;
}
long long f(long long i)
{
return bswap64(i);
}
constexpr long long bswapD(double x)
{
return bswap64(*(long long*)&x);
}
long long g(double x)
{
return bswapD(x);
}
===============================================================
There are three observations / bugs:
1) bswapD is never recognized as byte-swapping
2) bswap64 is optimized to bswap at -O2 but not at -O3
3) 131t.bswap never shows bswap, apparently the pass doesn't detect this way of
writing bswap, leaving it to the RTL optimizers. Hence I classified this as
tree-optimization bug.
Verified at -O2 with 9.3, 10.1 and trunk on the compiler explorer.
I'm flagging this as a regression because at -O2 gcc 8.3 detects bswap in both
cases, but I'm guessing that this is by some accident. In 7.5 neither function
is compiled as bswap.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/96135] [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
@ 2020-07-10 7:19 ` rguenth at gcc dot gnu.org
2021-03-11 9:07 ` rguenth at gcc dot gnu.org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-07-10 7:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2020-07-10
Keywords| |missed-optimization
Target Milestone|--- |9.4
Ever confirmed|0 |1
Priority|P3 |P2
--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
The bswap pass doesn't handle these patterns at all (it doesn't look at
stores).
What does handle this case is store-merging which - on trunk - figures
bswap in f() but not in g() likely because of the BIT_FIELD_REF ->
cast folding which makes the stores appear inhomogenous:
_3 = VIEW_CONVERT_EXPR<long long int>(x_2(D));
_4 = BIT_FIELD_REF <x_2(D), 8, 56>;
v.c[0] = _4;
...
_11 = (char) _3;
v.c[7] = _11;
store-merging already handles the cast vs. BIT_FIELD_REF case for f() but
it appearantly doesn't consider to look through a VIEW_CONVERT.
With -O3 we vectorize this in an inconvenient way and fully elide the store
so store-merging isn't the correct pass to handle this:
_3 = BIT_FIELD_REF <i_2(D), 8, 56>;
_4 = BIT_FIELD_REF <i_2(D), 8, 48>;
_5 = BIT_FIELD_REF <i_2(D), 8, 40>;
_6 = BIT_FIELD_REF <i_2(D), 8, 32>;
_7 = BIT_FIELD_REF <i_2(D), 8, 24>;
_8 = BIT_FIELD_REF <i_2(D), 8, 16>;
_9 = BIT_FIELD_REF <i_2(D), 8, 8>;
_10 = (char) i_2(D);
_21 = {_3, _4, _5, _6, _7, _8, _9, _10};
_18 = VIEW_CONVERT_EXPR<long long int>(_21);
v ={v} {CLOBBER};
return _18;
the vectorizer is also confused about BIT_FIELD_REF vs. cast here
(I repeatedly thought of removing that simplification ... but the
user could have written it as well :/). And it would look for
a vector function argument but that's something that could be fixed.
The above is all for GCC 10. GCC 8 possibly was lucky and did not have
that BIT_FIELD_REF -> cast simplification.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/96135] [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
2020-07-10 7:19 ` [Bug tree-optimization/96135] " rguenth at gcc dot gnu.org
@ 2021-03-11 9:07 ` rguenth at gcc dot gnu.org
2021-03-31 9:13 ` rguenth at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-03-11 9:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 50361
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50361&action=edit
WIP
On current trunk at -O3 f() again works via store-merging / vectorizing:
- _21 = {_3, _4, _5, _6, _7, _8, _9, _10};
+ bswapsrc_22 = (long unsigned int) i_2(D);
+ bswapdst_19 = __builtin_bswap64 (bswapsrc_22);
+ _21 = VIEW_CONVERT_EXPR<vector(8) char>(bswapdst_19);
but g() does not, because init_symbolic_number doesn't like non-integral types.
Fixing that generates
_Z1gd:
.LFB2:
.cfi_startproc
movq %xmm0, %rax
bswap %rax
ret
but with -m32 it has the issue that we bswap only the lower part since
vectorizing produced two vector CTORs. So we'd need to use a BIT_FIELD_REF
to extract the integer representation.
WIP patch attached.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/96135] [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
2020-07-10 7:19 ` [Bug tree-optimization/96135] " rguenth at gcc dot gnu.org
2021-03-11 9:07 ` rguenth at gcc dot gnu.org
@ 2021-03-31 9:13 ` rguenth at gcc dot gnu.org
2021-06-01 8:18 ` [Bug tree-optimization/96135] [9/10/11/12 " rguenth at gcc dot gnu.org
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-03-31 9:13 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
See also PR96573
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/96135] [9/10/11/12 regression] bswap not detected by bswap pass, unexpected results between optimization levels
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
` (2 preceding siblings ...)
2021-03-31 9:13 ` rguenth at gcc dot gnu.org
@ 2021-06-01 8:18 ` rguenth at gcc dot gnu.org
2021-09-08 10:48 ` pinskia at gcc dot gnu.org
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-06-01 8:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|9.4 |9.5
--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9.4 is being released, retargeting bugs to GCC 9.5.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/96135] [9/10/11/12 regression] bswap not detected by bswap pass, unexpected results between optimization levels
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
` (3 preceding siblings ...)
2021-06-01 8:18 ` [Bug tree-optimization/96135] [9/10/11/12 " rguenth at gcc dot gnu.org
@ 2021-09-08 10:48 ` pinskia at gcc dot gnu.org
2021-09-08 10:58 ` pinskia at gcc dot gnu.org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-09-08 10:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note on the trunk for f and g at -O3 -msse4 (and -O3 on aarch64), GCC produces:
_21 = VIEW_CONVERT_EXPR<vector(8) char>(i_2(D));
_22 = VEC_PERM_EXPR <_21, _21, { 7, 6, 5, 4, 3, 2, 1, 0 }>;
_18 = VIEW_CONVERT_EXPR<long long int>(_22);
But that VEC_PERM_EXPR is a bswap :).
So to fix this at -O3 -msse4, maybe we could just do:
(simplify
(view_convert (vec_perm @0 @0 vector_cst_byteswap_p @1))
(if (INTERGAL_TYPE_P (type)))
(convert (bswap (view_convert @1))))
Note I don't think we want to do the byteswap in the integer registers if we
are going back to the floating point registers.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/96135] [9/10/11/12 regression] bswap not detected by bswap pass, unexpected results between optimization levels
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
` (4 preceding siblings ...)
2021-09-08 10:48 ` pinskia at gcc dot gnu.org
@ 2021-09-08 10:58 ` pinskia at gcc dot gnu.org
2022-05-27 9:43 ` [Bug tree-optimization/96135] [10/11/12/13 " rguenth at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-09-08 10:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #5)
> Note on the trunk for f and g at -O3 -msse4 (and -O3 on aarch64), GCC
> produces:
> _21 = VIEW_CONVERT_EXPR<vector(8) char>(i_2(D));
> _22 = VEC_PERM_EXPR <_21, _21, { 7, 6, 5, 4, 3, 2, 1, 0 }>;
> _18 = VIEW_CONVERT_EXPR<long long int>(_22);
>
> But that VEC_PERM_EXPR is a bswap :).
>
> So to fix this at -O3 -msse4, maybe we could just do:
> (simplify
> (view_convert (vec_perm @0 @0 vector_cst_byteswap_p @1))
> (if (INTERGAL_TYPE_P (type)))
> (convert (bswap (view_convert @1))))
>
> Note I don't think we want to do the byteswap in the integer registers if we
> are going back to the floating point registers.
Or we could just support that in vect-lowering.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/96135] [10/11/12/13 regression] bswap not detected by bswap pass, unexpected results between optimization levels
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
` (5 preceding siblings ...)
2021-09-08 10:58 ` pinskia at gcc dot gnu.org
@ 2022-05-27 9:43 ` rguenth at gcc dot gnu.org
2022-06-28 10:41 ` jakub at gcc dot gnu.org
2023-07-07 10:37 ` [Bug tree-optimization/96135] [11/12/13/14 " rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-05-27 9:43 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|9.5 |10.4
--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9 branch is being closed
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/96135] [10/11/12/13 regression] bswap not detected by bswap pass, unexpected results between optimization levels
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
` (6 preceding siblings ...)
2022-05-27 9:43 ` [Bug tree-optimization/96135] [10/11/12/13 " rguenth at gcc dot gnu.org
@ 2022-06-28 10:41 ` jakub at gcc dot gnu.org
2023-07-07 10:37 ` [Bug tree-optimization/96135] [11/12/13/14 " rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-06-28 10:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|10.4 |10.5
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/96135] [11/12/13/14 regression] bswap not detected by bswap pass, unexpected results between optimization levels
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
` (7 preceding siblings ...)
2022-06-28 10:41 ` jakub at gcc dot gnu.org
@ 2023-07-07 10:37 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-07-07 10:37 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96135
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|10.5 |11.5
--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-07-07 10:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 14:01 [Bug tree-optimization/96135] New: [9/10/11 regression] bswap not detected by bswap pass, unexpected results between optimization levels tobi at gcc dot gnu.org
2020-07-10 7:19 ` [Bug tree-optimization/96135] " rguenth at gcc dot gnu.org
2021-03-11 9:07 ` rguenth at gcc dot gnu.org
2021-03-31 9:13 ` rguenth at gcc dot gnu.org
2021-06-01 8:18 ` [Bug tree-optimization/96135] [9/10/11/12 " rguenth at gcc dot gnu.org
2021-09-08 10:48 ` pinskia at gcc dot gnu.org
2021-09-08 10:58 ` pinskia at gcc dot gnu.org
2022-05-27 9:43 ` [Bug tree-optimization/96135] [10/11/12/13 " rguenth at gcc dot gnu.org
2022-06-28 10:41 ` jakub at gcc dot gnu.org
2023-07-07 10:37 ` [Bug tree-optimization/96135] [11/12/13/14 " rguenth 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).