public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask
@ 2022-04-19 20:19 christophm30 at gmail dot com
2022-04-19 20:23 ` [Bug tree-optimization/105314] " christophm30 at gmail dot com
` (12 more replies)
0 siblings, 13 replies; 14+ messages in thread
From: christophm30 at gmail dot com @ 2022-04-19 20:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
Bug ID: 105314
Summary: ifcvt regression in noce_try_store_flag_mask
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: christophm30 at gmail dot com
Target Milestone: ---
The function noce_try_store_flag_mask() in ifcvt.cc converts
"if (test) x = 0;" to "x &= -(test == 0);" (if costs permit that).
Commit 3a7ba8fd (which was introduced a month ago to fix PR104960) triggers a
regression so that the if-conversion can't be performed anymore.
On RISC-V this manifests as follows:
"""
long func(long a, long b, long c)
{
if (c)
a = 0;
return a;
}
"""
Old code:
0000000000000000 <func>:
0: 00163613 seqz a2,a2
4: 40c00633 neg a2,a2
8: 8d71 and a0,a0,a2
a: 8082 ret
New (branching) code:
0000000000000000 <func>:
0: c211 beqz a2,4 <.L1>
2: 4501 li a0,0
0000000000000004 <.L1>:
4: 8082 ret
Looking through the test suite, I could only find the file
"gcc.target/arm/ifcvt-size-check.c" which should be affected by this regression
as well. However, I haven't tested that.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug tree-optimization/105314] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
@ 2022-04-19 20:23 ` christophm30 at gmail dot com
2022-04-20 7:10 ` marxin at gcc dot gnu.org
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: christophm30 at gmail dot com @ 2022-04-19 20:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
--- Comment #1 from Christoph Müllner <christophm30 at gmail dot com> ---
A first analysis in noce_try_store_flag_mask() showed the following observation
in case of the failing conversion into the branchless sequence:
* if_info->b == const0_rtx holds
* rtx_equal_p (if_info->a, if_info->x) does not hold
Assuming that the generic form would be "x := c ? x : 0", the second
observation seems odd.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug tree-optimization/105314] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
2022-04-19 20:23 ` [Bug tree-optimization/105314] " christophm30 at gmail dot com
@ 2022-04-20 7:10 ` marxin at gcc dot gnu.org
2022-04-20 7:10 ` marxin at gcc dot gnu.org
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-04-20 7:10 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |marxin at gcc dot gnu.org
--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
> Commit 3a7ba8fd (which was introduced a month ago to fix PR104960) triggers
g:3a7ba8fd
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug tree-optimization/105314] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
2022-04-19 20:23 ` [Bug tree-optimization/105314] " christophm30 at gmail dot com
2022-04-20 7:10 ` marxin at gcc dot gnu.org
@ 2022-04-20 7:10 ` marxin at gcc dot gnu.org
2022-04-20 7:49 ` [Bug target/105314] " rguenth at gcc dot gnu.org
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-04-20 7:10 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Ever confirmed|0 |1
Last reconfirmed| |2022-04-20
CC| |rguenth at gcc dot gnu.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/105314] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (2 preceding siblings ...)
2022-04-20 7:10 ` marxin at gcc dot gnu.org
@ 2022-04-20 7:49 ` rguenth at gcc dot gnu.org
2022-04-22 9:48 ` [Bug target/105314] [12 Regression] " pinskia at gcc dot gnu.org
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-04-20 7:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Component|tree-optimization |target
Keywords| |missed-optimization
Target| |riscv
--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
I observed sth similar on arm and x86_64 where the backends were adjusted to
treat two equal variants the same. The issue that is observed from the
GIMPLE level change is that the edge containing the forwarder changes and
so we get slightly different initial RTL out of it.
You have to analyze in more detail what happens.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug target/105314] [12 Regression] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (3 preceding siblings ...)
2022-04-20 7:49 ` [Bug target/105314] " rguenth at gcc dot gnu.org
@ 2022-04-22 9:48 ` pinskia at gcc dot gnu.org
2022-04-25 15:45 ` [Bug rtl-optimization/105314] " jakub at gcc dot gnu.org
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-04-22 9:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|ifcvt regression in |[12 Regression] ifcvt
|noce_try_store_flag_mask |regression in
| |noce_try_store_flag_mask
Target Milestone|--- |12.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug rtl-optimization/105314] [12 Regression] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (4 preceding siblings ...)
2022-04-22 9:48 ` [Bug target/105314] [12 Regression] " pinskia at gcc dot gnu.org
@ 2022-04-25 15:45 ` jakub at gcc dot gnu.org
2022-04-25 22:34 ` ptomsich at gcc dot gnu.org
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-04-25 15:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52869
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52869&action=edit
gcc12-pr105314.patch
Untested fix.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug rtl-optimization/105314] [12 Regression] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (5 preceding siblings ...)
2022-04-25 15:45 ` [Bug rtl-optimization/105314] " jakub at gcc dot gnu.org
@ 2022-04-25 22:34 ` ptomsich at gcc dot gnu.org
2022-04-25 22:40 ` christophm30 at gmail dot com
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: ptomsich at gcc dot gnu.org @ 2022-04-25 22:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
--- Comment #5 from ptomsich at gcc dot gnu.org ---
The fix addresses the issue and generates no new failures on small test cases.
Testing against SPEC is still ongoing and I'll report back once that has
completed.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug rtl-optimization/105314] [12 Regression] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (6 preceding siblings ...)
2022-04-25 22:34 ` ptomsich at gcc dot gnu.org
@ 2022-04-25 22:40 ` christophm30 at gmail dot com
2022-04-25 22:47 ` ptomsich at gcc dot gnu.org
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: christophm30 at gmail dot com @ 2022-04-25 22:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
--- Comment #6 from Christoph Müllner <christophm30 at gmail dot com> ---
The proposed fix is triggering an invalid transformation.
The pattern we need to transform is:
Convert "if (test) x = 0;" to "x &= -(test == 0);"
If there is an else branch, we can't apply the transformation.
Error case:
"""
long func2 (long a, long b, long c) // should not be optimized
{
if (c)
a = 0;
else
a = 5;
return a;
}
"""
With the patch this gets compiled to:
0000000000000006 <func2>:
6: 00163513 seqz a0,a2
a: 40a00533 neg a0,a0
e: 8915 andi a0,a0,5
10: 8082 ret
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug rtl-optimization/105314] [12 Regression] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (7 preceding siblings ...)
2022-04-25 22:40 ` christophm30 at gmail dot com
@ 2022-04-25 22:47 ` ptomsich at gcc dot gnu.org
2022-04-26 0:05 ` christophm30 at gmail dot com
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: ptomsich at gcc dot gnu.org @ 2022-04-25 22:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
--- Comment #7 from ptomsich at gcc dot gnu.org ---
The transformation for
"""
long func2 (long a, long b, long c)
{
if (c)
a = 0;
else
a = 5;
return a;
}
"""
into
"""
0000000000000006 <func2>:
6: 00163513 seqz a0,a2
a: 40a00533 neg a0,a0
e: 8915 andi a0,a0,5
10: 8082 ret
"""
is correct, as we get
"""
tmp = c ? 0 : -1;
a = tmp & 5;
"""
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug rtl-optimization/105314] [12 Regression] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (8 preceding siblings ...)
2022-04-25 22:47 ` ptomsich at gcc dot gnu.org
@ 2022-04-26 0:05 ` christophm30 at gmail dot com
2022-04-26 8:12 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: christophm30 at gmail dot com @ 2022-04-26 0:05 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
--- Comment #8 from Christoph Müllner <christophm30 at gmail dot com> ---
Yes, I was wrong in my previous comment.
Jakub's patch is of course right.
The transformation in noce_try_store_flag_mask() does:
x = cond ? 0 else b // b may be x
==>
target = cond ? 0 : -1; // seqz & neg
x = target & b; // and
I was misled by the comment above the function,
which only shows the special case when b equals x.
Probably this is a better comment:
/* Convert "if (test) x = 0 else x = y;" to "x = -(test == 0) & y;" */
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug rtl-optimization/105314] [12 Regression] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (9 preceding siblings ...)
2022-04-26 0:05 ` christophm30 at gmail dot com
@ 2022-04-26 8:12 ` cvs-commit at gcc dot gnu.org
2022-04-26 8:13 ` jakub at gcc dot gnu.org
2024-01-26 21:48 ` cvs-commit at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-04-26 8:12 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:7d31c678d68d7b6820a958584619ca763b0eb9c5
commit r12-8264-g7d31c678d68d7b6820a958584619ca763b0eb9c5
Author: Jakub Jelinek <jakub@redhat.com>
Date: Tue Apr 26 10:11:58 2022 +0200
ifcvt: Improve noce_try_store_flag_mask [PR105314]
The following testcase regressed on riscv due to the splitting of critical
edges in the sink pass, similarly to x86_64 compared to GCC 11 we now swap
the edges, whether true or false edge goes to an empty forwarded bb.
From GIMPLE POV, those 2 forms are equivalent, but as can be seen here, for
some ifcvt opts it matters one way or another.
On this testcase, noce_try_store_flag_mask used to trigger and transformed
if (pseudo2) pseudo1 = 0;
into
pseudo1 &= -(pseudo2 == 0);
But with the swapped edges ifcvt actually sees
if (!pseudo2) pseudo3 = pseudo1; else pseudo3 = 0;
and noce_try_store_flag_mask punts. IMHO there is no reason why it
should punt those, it is equivalent to
pseudo3 = pseudo1 & -(pseudo2 == 0);
and especially if the target has 3 operand AND, it shouldn't be any more
costly (and even with 2 operand AND, it might very well happen that RA
can make it happen without any extra moves).
Initially I've just removed the rtx_equal_p calls from the conditions
and didn't add anything there, but that broke aarch64 bootstrap and
regressed some testcases on x86_64, where if_info->a or if_info->b could be
some larger expression that we can't force into a register.
Furthermore, the case where both if_info->a and if_info->b are constants is
better handled by other ifcvt optimizations like noce_try_store_flag
or noce_try_inverse_constants or noce_try_store_flag_constants.
So, I've restricted it to just a REG (perhaps SUBREG of REG might be ok
too)
next to what has been handled previously.
2022-04-26 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/105314
* ifcvt.cc (noce_try_store_flag_mask): Don't require that the
non-zero
operand is equal to if_info->x, instead use the non-zero operand
as one of the operands of AND with if_info->x as target.
* gcc.target/riscv/pr105314.c: New test.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug rtl-optimization/105314] [12 Regression] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (10 preceding siblings ...)
2022-04-26 8:12 ` cvs-commit at gcc dot gnu.org
@ 2022-04-26 8:13 ` jakub at gcc dot gnu.org
2024-01-26 21:48 ` cvs-commit at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-04-26 8:13 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Hopefully fixed.
The 2 constant cases are handled in other noce_try* routines.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Bug rtl-optimization/105314] [12 Regression] ifcvt regression in noce_try_store_flag_mask
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
` (11 preceding siblings ...)
2022-04-26 8:13 ` jakub at gcc dot gnu.org
@ 2024-01-26 21:48 ` cvs-commit at gcc dot gnu.org
12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-01-26 21:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105314
--- Comment #11 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Maciej W. Rozycki <macro@gcc.gnu.org>:
https://gcc.gnu.org/g:3e3b9b708d390074f7825401b808e0ed41552c1d
commit r14-8459-g3e3b9b708d390074f7825401b808e0ed41552c1d
Author: Maciej W. Rozycki <macro@embecosm.com>
Date: Fri Jan 26 21:47:40 2024 +0000
RISC-V/testsuite: Also verify if-conversion runs for pr105314.c
Verify that if-conversion succeeded through noce_try_store_flag_mask, as
per PR rtl-optimization/105314, tightening the test case and making it
explicit.
gcc/testsuite/
* gcc.target/riscv/pr105314.c: Scan the RTL "ce1" pass too.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-01-26 21:48 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-19 20:19 [Bug tree-optimization/105314] New: ifcvt regression in noce_try_store_flag_mask christophm30 at gmail dot com
2022-04-19 20:23 ` [Bug tree-optimization/105314] " christophm30 at gmail dot com
2022-04-20 7:10 ` marxin at gcc dot gnu.org
2022-04-20 7:10 ` marxin at gcc dot gnu.org
2022-04-20 7:49 ` [Bug target/105314] " rguenth at gcc dot gnu.org
2022-04-22 9:48 ` [Bug target/105314] [12 Regression] " pinskia at gcc dot gnu.org
2022-04-25 15:45 ` [Bug rtl-optimization/105314] " jakub at gcc dot gnu.org
2022-04-25 22:34 ` ptomsich at gcc dot gnu.org
2022-04-25 22:40 ` christophm30 at gmail dot com
2022-04-25 22:47 ` ptomsich at gcc dot gnu.org
2022-04-26 0:05 ` christophm30 at gmail dot com
2022-04-26 8:12 ` cvs-commit at gcc dot gnu.org
2022-04-26 8:13 ` jakub at gcc dot gnu.org
2024-01-26 21:48 ` cvs-commit 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).