public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs
@ 2022-03-01 11:02 ro at gcc dot gnu.org
2022-03-01 11:04 ` [Bug target/104732] " ro at gcc dot gnu.org
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: ro at gcc dot gnu.org @ 2022-03-01 11:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104732
Bug ID: 104732
Summary: gcc.target/i386/pr100711-1.c FAILs
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: ro at gcc dot gnu.org
CC: sayle at gcc dot gnu.org
Target Milestone: ---
Target: i?86-pc-solaris2.11, x86_64-pc-solaris2.11
The gcc.target/i386/pr100711-1.c test FAILs on 32-bit Solaris/x86:
FAIL: gcc.target/i386/pr100711-1.c scan-assembler-times pandn 2
The pattern occurs only once. Attaching assembler output.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/104732] gcc.target/i386/pr100711-1.c FAILs
2022-03-01 11:02 [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs ro at gcc dot gnu.org
@ 2022-03-01 11:04 ` ro at gcc dot gnu.org
2022-03-01 11:04 ` ro at gcc dot gnu.org
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: ro at gcc dot gnu.org @ 2022-03-01 11:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104732
--- Comment #1 from Rainer Orth <ro at gcc dot gnu.org> ---
Created attachment 52534
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52534&action=edit
32-bit i386-pc-solaris2.11 pr100711-1.s
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/104732] gcc.target/i386/pr100711-1.c FAILs
2022-03-01 11:02 [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs ro at gcc dot gnu.org
2022-03-01 11:04 ` [Bug target/104732] " ro at gcc dot gnu.org
@ 2022-03-01 11:04 ` ro at gcc dot gnu.org
2022-03-01 13:57 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: ro at gcc dot gnu.org @ 2022-03-01 11:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104732
Rainer Orth <ro at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |12.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/104732] gcc.target/i386/pr100711-1.c FAILs
2022-03-01 11:02 [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs ro at gcc dot gnu.org
2022-03-01 11:04 ` [Bug target/104732] " ro at gcc dot gnu.org
2022-03-01 11:04 ` ro at gcc dot gnu.org
@ 2022-03-01 13:57 ` jakub at gcc dot gnu.org
2022-03-01 15:23 ` ro at CeBiTec dot Uni-Bielefeld.DE
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-01 13:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104732
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The reason is that in order to match pandn in bar, it relies on the stv pass
being done, and TARGET_STV is disabled if -mpreferred-stack-boundary={2,3}, or
-mincoming-stack-boundatry={2,3} or -mstackrealign. The last one is what
matters on Solaris (and on cygming too?), because those define
STACK_REALIGN_DEFAULT to non-zero.
I'd say we should just add -mno-stackrealign to dg-options in that testcase.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/104732] gcc.target/i386/pr100711-1.c FAILs
2022-03-01 11:02 [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs ro at gcc dot gnu.org
` (2 preceding siblings ...)
2022-03-01 13:57 ` jakub at gcc dot gnu.org
@ 2022-03-01 15:23 ` ro at CeBiTec dot Uni-Bielefeld.DE
2022-03-04 12:16 ` [Bug testsuite/104732] " roger at nextmovesoftware dot com
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: ro at CeBiTec dot Uni-Bielefeld.DE @ 2022-03-01 15:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104732
--- Comment #3 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
[...]
> I'd say we should just add -mno-stackrealign to dg-options in that testcase.
That works indeed. We already had several instances of this issue in
the past.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug testsuite/104732] gcc.target/i386/pr100711-1.c FAILs
2022-03-01 11:02 [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs ro at gcc dot gnu.org
` (3 preceding siblings ...)
2022-03-01 15:23 ` ro at CeBiTec dot Uni-Bielefeld.DE
@ 2022-03-04 12:16 ` roger at nextmovesoftware dot com
2022-03-05 8:52 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: roger at nextmovesoftware dot com @ 2022-03-04 12:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104732
Roger Sayle <roger at nextmovesoftware dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |roger at nextmovesoftware dot com
Last reconfirmed| |2022-03-04
Ever confirmed|0 |1
Status|UNCONFIRMED |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |roger at nextmovesoftware dot com
--- Comment #4 from Roger Sayle <roger at nextmovesoftware dot com> ---
Mine. Currently boostrapping and regression testing Uros' suggested
improvements to my patch at
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591199.html.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug testsuite/104732] gcc.target/i386/pr100711-1.c FAILs
2022-03-01 11:02 [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs ro at gcc dot gnu.org
` (4 preceding siblings ...)
2022-03-04 12:16 ` [Bug testsuite/104732] " roger at nextmovesoftware dot com
@ 2022-03-05 8:52 ` cvs-commit at gcc dot gnu.org
2022-03-09 16:07 ` roger at nextmovesoftware dot com
2022-03-11 8:59 ` ro at CeBiTec dot Uni-Bielefeld.DE
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-03-05 8:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104732
--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Roger Sayle <sayle@gcc.gnu.org>:
https://gcc.gnu.org/g:8ea4a34bd0b0a46277b5e077c89cbd86dfb09c48
commit r12-7502-g8ea4a34bd0b0a46277b5e077c89cbd86dfb09c48
Author: Roger Sayle <roger@nextmovesoftware.com>
Date: Sat Mar 5 08:50:45 2022 +0000
PR 104732: Simplify/fix DI mode logic expansion/splitting on -m32.
This clean-up patch resolves PR testsuite/104732, the failure of the recent
test gcc.target/i386/pr100711-1.c on 32-bit Solaris/x86. Rather than just
tweak the testcase, the proposed approach is to fix the underlying problem
by removing the "TARGET_STV && TARGET_SSE2" conditionals from the DI mode
logical operation expanders and pre-reload splitters in i386.md, which as
I'll show generate inferior code (even a GCC 12 regression) on
!TARGET_64BIT
whenever -mno-stv (such as Solaris) or -msse (but not -msse2).
First a little bit of history. In the beginning, DImode operations on
i386 weren't defined by the machine description, and lowered during RTL
expansion to SI mode operations. The with PR 65105 in 2015, -mstv was
added, together with a SWIM1248x mode iterator (later renamed to SWIM1248x)
together with several *<code>di3_doubleword post-reload splitters that
made use of register allocation to perform some double word operations
in 64-but XMM registers. A short while later in 2016, PR 70322 added
similar support for one_cmpldi2. All of this logic was dependent upon
"!TARGET_64BIT && TARGET_STV && TARGET_SSE2". With the passing of time,
these conditions became irrelevant when in 2019, it was decided to split
these double-word patterns before reload.
https://gcc.gnu.org/pipermail/gcc-patches/2019-June/523877.html
https://gcc.gnu.org/pipermail/gcc-patches/2019-October/532236.html
Hence the current situation, where on most modern CPU architectures
(where "TARGET_STV && TARGET_SSE2" is true), RTL is expanded with DI
mode operations, that are then split into two SI mode instructions
before reload, except on Solaris and other odd cases, where the splitting
is to two SI mode instructions is done during RTL expansion. By the
time compilation reaches register allocation both paths in theory
produce identical or similar code, so the vestigial legacy/logic would
appear to be harmless.
Unfortunately, there is one place where this arbitrary choice of how
to lower DI mode doubleword operations is visible to the middle-end,
it controls whether the backend appears to have a suitable optab, and
the presence (or not) of DImode optabs can influence vectorization
cost models and veclower decisions.
The issue (and code quality regression) can be seen in this test case:
typedef long long v2di __attribute__((vector_size (16)));
v2di x;
void foo (long long a)
{
v2di t = {a, a};
x = ~t;
}
which when compiled with "-O2 -m32 -msse -march=pentiumpro" produces:
foo: subl $28, %esp
movl %ebx, 16(%esp)
movl 32(%esp), %eax
movl %esi, 20(%esp)
movl 36(%esp), %edx
movl %edi, 24(%esp)
movl %eax, %esi
movl %eax, %edi
movl %edx, %ebx
movl %edx, %ecx
notl %esi
notl %ebx
movl %esi, (%esp)
notl %edi
notl %ecx
movl %ebx, 4(%esp)
movl 20(%esp), %esi
movl %edi, 8(%esp)
movl 16(%esp), %ebx
movl %ecx, 12(%esp)
movl 24(%esp), %edi
movss 8(%esp), %xmm1
movss 12(%esp), %xmm2
movss (%esp), %xmm0
movss 4(%esp), %xmm3
unpcklps %xmm2, %xmm1
unpcklps %xmm3, %xmm0
movlhps %xmm1, %xmm0
movaps %xmm0, x
addl $28, %esp
ret
Importantly notice the four "notl" instructions. With this patch:
foo: subl $28, %esp
movl 32(%esp), %edx
movl 36(%esp), %eax
notl %edx
movl %edx, (%esp)
notl %eax
movl %eax, 4(%esp)
movl %edx, 8(%esp)
movl %eax, 12(%esp)
movaps (%esp), %xmm1
movaps %xmm1, x
addl $28, %esp
ret
Notice only two "notl" instructions. Checking with godbolt.org, GCC
generated 4 NOTs in GCC 4.x and 5.x, 2 NOTs between GCC 6.x and 9.x,
and regressed to 4 NOTs since GCC 10.x [which hopefully qualifies
this clean-up as suitable for stage 4].
Most significantly, this patch allows pr100711-1.c to pass with
-mno-stv, allowing pandn to be used with V2DImode on Solaris/x86.
Fingers-crossed this should reduce the number of discrepancies
encountered supporting Solaris/x86.
2022-03-05 Roger Sayle <roger@nextmovesoftware.com>
Uroš Bizjak <ubizjak@gmail.com>
gcc/ChangeLog
PR testsuite/104732
* config/i386/i386.md (SWIM1248x): Renamed from SWIM1248s.
Include DI mode unconditionally.
(*anddi3_doubleword): Remove && TARGET_STV && TARGET_SSE2
condition,
i.e. always split on !TARGET_64BIT.
(*<any_or>di3_doubleword): Likewise.
(*one_cmpldi2_doubleword): Likewise.
(and<mode>3 expander): Update to use SWIM1248x from SWIM1248s.
(<any_or><mode>3 expander): Likewise.
(one_cmpl<mode>2 expander): Likewise.
gcc/testsuite/ChangeLog
PR testsuite/104732
* gcc.target/i386/pr104732.c: New test case.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug testsuite/104732] gcc.target/i386/pr100711-1.c FAILs
2022-03-01 11:02 [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs ro at gcc dot gnu.org
` (5 preceding siblings ...)
2022-03-05 8:52 ` cvs-commit at gcc dot gnu.org
@ 2022-03-09 16:07 ` roger at nextmovesoftware dot com
2022-03-11 8:59 ` ro at CeBiTec dot Uni-Bielefeld.DE
7 siblings, 0 replies; 9+ messages in thread
From: roger at nextmovesoftware dot com @ 2022-03-09 16:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104732
Roger Sayle <roger at nextmovesoftware dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #6 from Roger Sayle <roger at nextmovesoftware dot com> ---
This should now be fixed on mainline. Rainer please let me know if you notice
any remaining issues on solaris/x86.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug testsuite/104732] gcc.target/i386/pr100711-1.c FAILs
2022-03-01 11:02 [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs ro at gcc dot gnu.org
` (6 preceding siblings ...)
2022-03-09 16:07 ` roger at nextmovesoftware dot com
@ 2022-03-11 8:59 ` ro at CeBiTec dot Uni-Bielefeld.DE
7 siblings, 0 replies; 9+ messages in thread
From: ro at CeBiTec dot Uni-Bielefeld.DE @ 2022-03-11 8:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104732
--- Comment #7 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #6 from Roger Sayle <roger at nextmovesoftware dot com> ---
> This should now be fixed on mainline. Rainer please let me know if you notice
> any remaining issues on solaris/x86.
I've now run bootstraps with and without your patch: no regressions
apart from the usual flakey tests. Thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-11 8:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01 11:02 [Bug target/104732] New: gcc.target/i386/pr100711-1.c FAILs ro at gcc dot gnu.org
2022-03-01 11:04 ` [Bug target/104732] " ro at gcc dot gnu.org
2022-03-01 11:04 ` ro at gcc dot gnu.org
2022-03-01 13:57 ` jakub at gcc dot gnu.org
2022-03-01 15:23 ` ro at CeBiTec dot Uni-Bielefeld.DE
2022-03-04 12:16 ` [Bug testsuite/104732] " roger at nextmovesoftware dot com
2022-03-05 8:52 ` cvs-commit at gcc dot gnu.org
2022-03-09 16:07 ` roger at nextmovesoftware dot com
2022-03-11 8:59 ` ro at CeBiTec dot Uni-Bielefeld.DE
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).