public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug rtl-optimization/115182] New: [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095
@ 2024-05-22 1:49 hp at gcc dot gnu.org
2024-05-22 8:00 ` [Bug rtl-optimization/115182] " rguenth at gcc dot gnu.org
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: hp at gcc dot gnu.org @ 2024-05-22 1:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115182
Bug ID: 115182
Summary: [15 Regression] gcc.target/cris/pr93372-47.c at
r15-518-g99b1daae18c095
Product: gcc
Version: 15.0
Status: UNCONFIRMED
Keywords: missed-optimization, testsuite-fail
Severity: normal
Priority: P3
Component: rtl-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: hp at gcc dot gnu.org
CC: law at gcc dot gnu.org, pinskia at gcc dot gnu.org,
rguenth at gcc dot gnu.org, unassigned at gcc dot gnu.org
Depends on: 115144
Target Milestone: ---
Target: cris-elf
+++ This bug was initially created as a clone of Bug #115144 +++
This bug is only about the unfilled delay-slot that caused
gcc.target/cris/pr93372-47.c to fail starting at r15-518-g99b1daae18c095; not
about the incidental (possibly generic but at least unrelated to
delay-slot-filling) performance regression I noticed. The bug seems to be due
to a bug in reorg.cc or actually, in resource.cc.
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115144
[Bug 115144] [15 Regression] 2% performance regression for some codes with
r15-518-g99b1daae18c095
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug rtl-optimization/115182] [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095
2024-05-22 1:49 [Bug rtl-optimization/115182] New: [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095 hp at gcc dot gnu.org
@ 2024-05-22 8:00 ` rguenth at gcc dot gnu.org
2024-05-24 11:02 ` rguenth at gcc dot gnu.org
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-05-22 8:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115182
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |15.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug rtl-optimization/115182] [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095
2024-05-22 1:49 [Bug rtl-optimization/115182] New: [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095 hp at gcc dot gnu.org
2024-05-22 8:00 ` [Bug rtl-optimization/115182] " rguenth at gcc dot gnu.org
@ 2024-05-24 11:02 ` rguenth at gcc dot gnu.org
2024-05-25 3:41 ` hp at gcc dot gnu.org
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-05-24 11:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115182
Bug 115182 depends on bug 115144, which changed state.
Bug 115144 Summary: [15 Regression] 2% performance regression for some codes with r15-518-g99b1daae18c095
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115144
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug rtl-optimization/115182] [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095
2024-05-22 1:49 [Bug rtl-optimization/115182] New: [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095 hp at gcc dot gnu.org
2024-05-22 8:00 ` [Bug rtl-optimization/115182] " rguenth at gcc dot gnu.org
2024-05-24 11:02 ` rguenth at gcc dot gnu.org
@ 2024-05-25 3:41 ` hp at gcc dot gnu.org
2024-05-28 21:16 ` cvs-commit at gcc dot gnu.org
2024-05-29 10:52 ` hp at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: hp at gcc dot gnu.org @ 2024-05-25 3:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115182
Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2024-05-25
Status|UNCONFIRMED |ASSIGNED
Ever confirmed|0 |1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug rtl-optimization/115182] [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095
2024-05-22 1:49 [Bug rtl-optimization/115182] New: [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095 hp at gcc dot gnu.org
` (2 preceding siblings ...)
2024-05-25 3:41 ` hp at gcc dot gnu.org
@ 2024-05-28 21:16 ` cvs-commit at gcc dot gnu.org
2024-05-29 10:52 ` hp at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-05-28 21:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115182
--- Comment #1 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Hans-Peter Nilsson <hp@gcc.gnu.org>:
https://gcc.gnu.org/g:84b4ed45ea81ed5c4fb656a17846b26071c23e7d
commit r15-877-g84b4ed45ea81ed5c4fb656a17846b26071c23e7d
Author: Hans-Peter Nilsson <hp@axis.com>
Date: Tue May 28 23:15:57 2024 +0200
resource.cc (mark_target_live_regs): Don't look past target insn, PR115182
The PR115182 regression is that a delay-slot for a conditional branch,
is no longer filled with an insn that has been "sunk" because of
r15-518-g99b1daae18c095, for cris-elf w. -O2 -march=v10.
There are still sufficient "nearby" dependency-less insns that the
delay-slot shouldn't be empty. In particular there's one candidate in
the loop, right after an off-ramp branch, off the loop: a move from
$r9 to $r3.
beq .L2
nop
move.d $r9,$r3
But, the resource.cc data-flow-analysis incorrectly says it collides
with registers "live" at that .L2 off-ramp. The off-ramp insns
(inlined from simple_rand) look like this (left-to-right direction):
.L2:
move.d $r12,[_seed.0]
move.d $r13,[_seed.0+4]
ret
movem [$sp+],$r8
So, a store of a long long to _seed, a return instruction and a
restoring multi-register-load of r0..r8 (all callee-saved registers)
in the delay-slot of the return insn. The return-value is kept in
$r10,$r11 so in total $r10..$r13 live plus the stack-pointer and
return-address registers. But, mark_target_live_regs says that
$r0..$r8 are also live because it *includes the registers live for the
return instruction*! While they "come alive" after the movem, they
certainly aren't live at the "off-ramp" .L2 label.
The problem is in mark_target_live_regs: it consults a hash-table
indexed by insn uid, where it tracks the currently live registers with
a "generation" count to handle when it moves around insn, filling
delay-slots. As a fall-back, it starts with registers live at the
start of each basic block, calculated by the comparatively modern df
machinery (except that it can fail finding out which basic block an
insn belongs to, at which times it includes all registers film at 11),
and tracks the semantics of insns up to each insn.
You'd think that's all that should be done, but then for some reason
it *also* looks at insns *after the target insn* up to a few branches,
and includes that in the set of live registers! This is the code in
mark_target_live_regs that starts with the call to
find_dead_or_set_registers. I couldn't make sense of it, so I looked
at its history, and I think I found the cause; it's a thinko or
possibly two thinkos. The original implementation, gcc-git-described
as r0-97-g9c7e297806a27f, later moved from reorg.c to resource.c in
r0-20470-gca545bb569b756.
I believe the "extra" lookup was intended to counter flaws in the
reorg.c/resource.c register liveness analysis; to inspect insns along
the execution paths to exclude registers that, when looking at
subsequent insns, weren't live. That guess is backed by a sentence in
the updated (i.e. deleted) part of the function head comment for
mark_target_live_regs: "Next, scan forward from TARGET looking for
things set or clobbered before they are used. These are not live."
To me that sounds like flawed register-liveness data.
An epilogue expanded as RTX (i.e. not just assembly code emitted as
text) is introduced in basepoints/gcc-0-1334-gbdac5f5848fb, so before
that time, nobody would notice that saved registers were included as
live registers in delay-slots in "next-to-last" basic blocks.
Then in r0-24783-g96e9c98d59cc40, the intersection ("and") was changed
to a union ("or"), i.e. it added to the set of live registers instead
of thinning it out. In the gcc-patches archives, I see the patch
submission doesn't offer a C test-case and only has RTX snippets
(apparently for SPARC). The message does admit that the change goes
"against what the comments in the code say":
https://gcc.gnu.org/pipermail/gcc-patches/1999-November/021836.html
It looks like this was related to a bug with register liveness info
messed up when moving a "delay-slotted" insn from one slot to another.
But, I can't help but thinking it's just papering over a register
liveness bug elsewhere.
I think, with a reliable "DF_LR_IN", the whole thing *after* tracking
from start-of-bb up to the target insn should be removed; thus.
This patch also removes the now-unused find_dead_or_set_registers
function.
At r15-518, it fixes the issue for CRIS and improves coremark scores
at -O2 -march=v10 a tiny bit (about 0.05%).
PR rtl-optimization/115182
* resource.cc (mark_target_live_regs): Don't look for
unconditional branches after the target to improve on the
register liveness.
(find_dead_or_set_registers): Remove unused function.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug rtl-optimization/115182] [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095
2024-05-22 1:49 [Bug rtl-optimization/115182] New: [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095 hp at gcc dot gnu.org
` (3 preceding siblings ...)
2024-05-28 21:16 ` cvs-commit at gcc dot gnu.org
@ 2024-05-29 10:52 ` hp at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: hp at gcc dot gnu.org @ 2024-05-29 10:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115182
Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #2 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
Final verification: pr93372-47.c again passed in my autotester, at
r15-884-ge5fc5d42d25c86.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-29 10:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-22 1:49 [Bug rtl-optimization/115182] New: [15 Regression] gcc.target/cris/pr93372-47.c at r15-518-g99b1daae18c095 hp at gcc dot gnu.org
2024-05-22 8:00 ` [Bug rtl-optimization/115182] " rguenth at gcc dot gnu.org
2024-05-24 11:02 ` rguenth at gcc dot gnu.org
2024-05-25 3:41 ` hp at gcc dot gnu.org
2024-05-28 21:16 ` cvs-commit at gcc dot gnu.org
2024-05-29 10:52 ` hp 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).