public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2
@ 2021-04-01 8:44 zsojka at seznam dot cz
2021-04-01 9:01 ` [Bug target/99863] " rguenth at gcc dot gnu.org
` (23 more replies)
0 siblings, 24 replies; 25+ messages in thread
From: zsojka at seznam dot cz @ 2021-04-01 8:44 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
Bug ID: 99863
Summary: [10/11 Regression] wrong code with -O
-fno-tree-forwprop -mno-sse2
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Keywords: wrong-code
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: zsojka at seznam dot cz
Target Milestone: ---
Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu
Created attachment 50493
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50493&action=edit
reduced testcase
Output:
$ x86_64-pc-linux-gnu-gcc -O -fno-tree-forwprop -mno-sse2 testcase.c -Wno-psabi
$ ./a.out
Aborted
$ x86_64-pc-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r11-7937-20210331154556-geadf009b229-checking-yes-rtl-df-extra-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/11.0.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--with-cloog --with-ppl --with-isl --build=x86_64-pc-linux-gnu
--host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu
--with-ld=/usr/bin/x86_64-pc-linux-gnu-ld
--with-as=/usr/bin/x86_64-pc-linux-gnu-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r11-7937-20210331154556-geadf009b229-checking-yes-rtl-df-extra-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.0.1 20210331 (experimental) (GCC)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug target/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
@ 2021-04-01 9:01 ` rguenth at gcc dot gnu.org
2021-04-01 9:14 ` rguenth at gcc dot gnu.org
` (22 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-04-01 9:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Target Milestone|--- |10.3
Ever confirmed|0 |1
Keywords| |needs-bisection
Last reconfirmed| |2021-04-01
Priority|P3 |P2
--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed. It's a bit hard to follow what goes wrong (didn't yet spot it).
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug target/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
2021-04-01 9:01 ` [Bug target/99863] " rguenth at gcc dot gnu.org
@ 2021-04-01 9:14 ` rguenth at gcc dot gnu.org
2021-04-01 9:58 ` rguenth at gcc dot gnu.org
` (21 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-04-01 9:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Also time to fix this stupid veclower behavior:
_7 = (unsigned int) _14;
_5 = {_7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7};
- v512u32_0_16 = _5 * v512u32_0_15(D);
+ _53 = {_7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7};
+ _54 = BIT_FIELD_REF <_53, 32, 0>;
+ _55 = BIT_FIELD_REF <v512u32_0_15(D), 32, 0>;
+ _56 = _54 * _55;
+ _57 = {_7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7};
+ _58 = BIT_FIELD_REF <_57, 32, 32>;
+ _59 = BIT_FIELD_REF <v512u32_0_15(D), 32, 32>;
+ _60 = _58 * _59;
...
instead of
_5 = {_7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7};
- v512u32_0_16 = _5 * v512u32_0_15(D);
_54 = BIT_FIELD_REF <_5, 32, 0>;
...
or even better
_5 = {_7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7, _7};
- v512u32_0_16 = _5 * v512u32_0_15(D);
+ _55 = BIT_FIELD_REF <v512u32_0_15(D), 32, 0>;
+ _56 = _7 * _55;
it's all "fixed" later by CSE of course.
The error might be involving the clever handling of
- v256u8_r_17 = _8 + _19;
+ _117 = BIT_FIELD_REF <_8, 64, 0>;
+ _118 = BIT_FIELD_REF <_19, 64, 0>;
+ _119 = _117 ^ _118;
+ _120 = _118 & 9187201950435737471;
+ _121 = _117 & 9187201950435737471;
+ _122 = _119 & 9259542123273814144;
+ _123 = _120 + _121;
+ _124 = _122 ^ _123;
+ _125 = BIT_FIELD_REF <_8, 64, 64>;
+ _126 = BIT_FIELD_REF <_19, 64, 64>;
...
+ _149 = {_124, _132, _140, _148};
+ _150 = VIEW_CONVERT_EXPR<v256u8>(_149);
+ v256u8_r_17 = _150;
it's enough to -fdisable-tree-forwprop4 (forwprop after veclower) to make
the problem show up so it might be as well an RTL optimization issue.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug target/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
2021-04-01 9:01 ` [Bug target/99863] " rguenth at gcc dot gnu.org
2021-04-01 9:14 ` rguenth at gcc dot gnu.org
@ 2021-04-01 9:58 ` rguenth at gcc dot gnu.org
2021-04-01 10:02 ` [Bug target/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba marxin at gcc dot gnu.org
` (20 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-04-01 9:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 6da66985ad6..1f417a52702 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -5231,6 +5231,7 @@ gimplify_init_constructor (tree *expr_p, gimple_seq
*pre_p, gimple_seq *post_p,
TREE_TYPE (ce->value)))
TREE_STATIC (ctor) = 0;
}
+ recompute_constructor_flags (ctor);
if (!is_gimple_reg (TREE_OPERAND (*expr_p, 0)))
TREE_OPERAND (*expr_p, 1) = get_formal_tmp_var (ctor, pre_p);
}
simplifies the IL (vector lowering was supposed to simplify the element
extraction but GENERIC match.pd folding is confused about TREE_SIDE_EFFECTS
on the vector CTOR). It still nicely triggers the bug though.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug target/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (2 preceding siblings ...)
2021-04-01 9:58 ` rguenth at gcc dot gnu.org
@ 2021-04-01 10:02 ` marxin at gcc dot gnu.org
2021-04-01 10:24 ` rguenth at gcc dot gnu.org
` (19 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: marxin at gcc dot gnu.org @ 2021-04-01 10:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[10/11 Regression] wrong |[10/11 Regression] wrong
|code with -O |code with -O
|-fno-tree-forwprop |-fno-tree-forwprop
|-mno-sse2 |-mno-sse2 since
| |r10-7268-g529ea7d9596b26ba
CC| |law at gcc dot gnu.org,
| |marxin at gcc dot gnu.org
--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r10-7268-g529ea7d9596b26ba.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug target/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (3 preceding siblings ...)
2021-04-01 10:02 ` [Bug target/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba marxin at gcc dot gnu.org
@ 2021-04-01 10:24 ` rguenth at gcc dot gnu.org
2021-04-01 10:45 ` [Bug rtl-optimization/99863] " rguenth at gcc dot gnu.org
` (18 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-04-01 10:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #4)
> Started with r10-7268-g529ea7d9596b26ba.
Reverting on trunk fixes the issue. Good vs. bad assembly shows the likely
culprit:
shrq $32, %rax
- movq %rax, %r8
- testq %rdi, %rdi
+ movq %rax, %rdi
setne %al
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (4 preceding siblings ...)
2021-04-01 10:24 ` rguenth at gcc dot gnu.org
@ 2021-04-01 10:45 ` rguenth at gcc dot gnu.org
2021-04-01 10:47 ` cvs-commit at gcc dot gnu.org
` (17 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-04-01 10:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Component|target |rtl-optimization
--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #5)
> (In reply to Martin Liška from comment #4)
> > Started with r10-7268-g529ea7d9596b26ba.
>
> Reverting on trunk fixes the issue. Good vs. bad assembly shows the likely
> culprit:
>
> shrq $32, %rax
> - movq %rax, %r8
> - testq %rdi, %rdi
> + movq %rax, %rdi
> setne %al
And this is
(insn 6 3 7 2 (set (reg:CCZ 17 flags)
(compare:CCZ (reg/v:DI 177 [ u64_0 ])
(const_int 0 [0]))) "t.c":14:30 8 {*cmpdi_ccno_1}
(nil))
(insn 7 6 8 2 (set (reg:QI 179)
(eq:QI (reg:CCZ 17 flags)
(const_int 0 [0]))) "t.c":14:30 802 {*setcc_qi}
(expr_list:REG_DEAD (reg:CCZ 17 flags)
(nil)))
...
(insn 23 22 24 2 (set (reg:CCZ 17 flags)
(compare:CCZ (reg/v:DI 177 [ u64_0 ])
(const_int 0 [0]))) "t.c":15:19 8 {*cmpdi_ccno_1}
(nil))
(insn 24 23 25 2 (set (reg:QI 190)
(ne:QI (reg:CCZ 17 flags)
(const_int 0 [0]))) "t.c":15:19 802 {*setcc_qi}
(nil))
deferring deletion of insn with uid = 23.
likely somehow seeing the set as a noop move. trial is (reg:CCZ 17 flags)
but likely CSE doesn't track CC flag clobbers appropriately though there's
no other CC clobber insn visible at this point. At least CSE1 fails
to remove the REG_DEAD note on reg:CCZ in insn 7?
Later we happily insert CC clobbers inbetween - notably DSE1 (eh?!)
produces
(insn 166 12 168 2 (set (reg:SI 262)
(subreg:SI (reg:DI 182 [ foo0_v256u32_0 ]) 0)) "t.c":14:36 75
{*movsi_internal}
(nil))
(insn 168 166 167 2 (set (reg:DI 263)
(reg:DI 182 [ foo0_v256u32_0 ])) "t.c":14:36 74 {*movdi_internal}
(expr_list:REG_DEAD (reg:DI 182 [ foo0_v256u32_0 ])
(nil)))
(insn 167 168 169 2 (parallel [
(set (reg:DI 263)
(lshiftrt:DI (reg:DI 263)
(const_int 32 [0x20])))
(clobber (reg:CC 17 flags))
]) "t.c":14:36 703 {*lshrdi3_1}
(expr_list:REG_UNUSED (reg:CC 17 flags)
(nil)))
which uses subregs/shifts to elide a load/store pair (I think).
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (5 preceding siblings ...)
2021-04-01 10:45 ` [Bug rtl-optimization/99863] " rguenth at gcc dot gnu.org
@ 2021-04-01 10:47 ` cvs-commit at gcc dot gnu.org
2021-04-01 11:16 ` ebotcazou at gcc dot gnu.org
` (16 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-01 10:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:
https://gcc.gnu.org/g:512429a885e87bef51057a001681b7d8d106e807
commit r11-7949-g512429a885e87bef51057a001681b7d8d106e807
Author: Richard Biener <rguenther@suse.de>
Date: Thu Apr 1 11:51:33 2021 +0200
tree-optimization/99863 - clear vector CTOR TREE_SIDE_EFFECTS
When we gimplify a vector CTOR the original CONSTRUCTOR tree remains
but we fail to recompute flags such as TREE_SIDE_EFFECTS. This causes
later GENERIC folding of them in vector lowering to give up since
the match.pd machinery is careful about TREE_SIDE_EFFECTS.
Fixing this makes vector lowering produce much less garbage and
thus following the IL for PR99793 easier.
2021-04-01 Richard Biener <rguenther@suse.de>
PR tree-optimization/99863
* gimplify.c (gimplify_init_constructor): Recompute vector
constructor flags.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (6 preceding siblings ...)
2021-04-01 10:47 ` cvs-commit at gcc dot gnu.org
@ 2021-04-01 11:16 ` ebotcazou at gcc dot gnu.org
2021-04-01 11:28 ` ebotcazou at gcc dot gnu.org
` (15 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2021-04-01 11:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rguenth at gcc dot gnu.org
--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Richard, did you mean to CC me for another PR by any chance?
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (7 preceding siblings ...)
2021-04-01 11:16 ` ebotcazou at gcc dot gnu.org
@ 2021-04-01 11:28 ` ebotcazou at gcc dot gnu.org
2021-04-01 12:26 ` jakub at gcc dot gnu.org
` (14 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2021-04-01 11:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #9 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Richard, did you mean to CC me for another PR by any chance?
Never mind, I was confused by your commit.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (8 preceding siblings ...)
2021-04-01 11:28 ` ebotcazou at gcc dot gnu.org
@ 2021-04-01 12:26 ` jakub at gcc dot gnu.org
2021-04-01 12:45 ` rguenther at suse dot de
` (13 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-01 12:26 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the REG_DEAD note isn't the problem here.
At least, when I dump bb 2 in rest_of_handle_dse right before df_analyze call,
there is
(insn 6 3 7 2 (set (reg:CCZ 17 flags)
(compare:CCZ (reg/v:DI 203 [ y ])
(const_int 0 [0]))) "pr99863.c":14:12 8 {*cmpdi_ccno_1}
(nil))
(insn 7 6 8 2 (set (reg:QI 205)
(eq:QI (reg:CCZ 17 flags)
(const_int 0 [0]))) "pr99863.c":14:12 802 {*setcc_qi}
(expr_list:REG_DEAD (reg:CCZ 17 flags)
(nil)))
...
(insn 40 38 41 2 (set (reg:QI 220)
(ne:QI (reg:CCZ 17 flags)
(const_int 0 [0]))) "pr99863.c":15:11 802 {*setcc_qi}
(expr_list:REG_DEAD (reg:CCZ 17 flags)
(nil)))
but if I dump it right after df_analyze call, I see:
(insn 6 3 7 2 (set (reg:CCZ 17 flags)
(compare:CCZ (reg/v:DI 203 [ y ])
(const_int 0 [0]))) "pr99863.c":14:12 8 {*cmpdi_ccno_1}
(expr_list:REG_DEAD (reg/v:DI 203 [ y ])
(nil)))
(insn 7 6 8 2 (set (reg:QI 205)
(eq:QI (reg:CCZ 17 flags)
(const_int 0 [0]))) "pr99863.c":14:12 802 {*setcc_qi}
(nil))
...
(insn 40 38 41 2 (set (reg:QI 220)
(ne:QI (reg:CCZ 17 flags)
(const_int 0 [0]))) "pr99863.c":15:11 802 {*setcc_qi}
(expr_list:REG_DEAD (reg:CCZ 17 flags)
(nil)))
i.e. the problematic REG_DEAD note is gone and I think at this point DF knows
that CC is live there.
But replace_read has code to check for live hard regs, see
note_stores (this_insn, look_for_hardregs, regs_set);
in there etc.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (9 preceding siblings ...)
2021-04-01 12:26 ` jakub at gcc dot gnu.org
@ 2021-04-01 12:45 ` rguenther at suse dot de
2021-04-01 13:01 ` jakub at gcc dot gnu.org
` (12 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: rguenther at suse dot de @ 2021-04-01 12:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #11 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 1 Apr 2021, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
>
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
>
> What |Removed |Added
> ----------------------------------------------------------------------------
> CC| |jakub at gcc dot gnu.org
>
> --- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> I think the REG_DEAD note isn't the problem here.
> At least, when I dump bb 2 in rest_of_handle_dse right before df_analyze call,
> there is
> (insn 6 3 7 2 (set (reg:CCZ 17 flags)
> (compare:CCZ (reg/v:DI 203 [ y ])
> (const_int 0 [0]))) "pr99863.c":14:12 8 {*cmpdi_ccno_1}
> (nil))
> (insn 7 6 8 2 (set (reg:QI 205)
> (eq:QI (reg:CCZ 17 flags)
> (const_int 0 [0]))) "pr99863.c":14:12 802 {*setcc_qi}
> (expr_list:REG_DEAD (reg:CCZ 17 flags)
> (nil)))
> ...
> (insn 40 38 41 2 (set (reg:QI 220)
> (ne:QI (reg:CCZ 17 flags)
> (const_int 0 [0]))) "pr99863.c":15:11 802 {*setcc_qi}
> (expr_list:REG_DEAD (reg:CCZ 17 flags)
> (nil)))
> but if I dump it right after df_analyze call, I see:
> (insn 6 3 7 2 (set (reg:CCZ 17 flags)
> (compare:CCZ (reg/v:DI 203 [ y ])
> (const_int 0 [0]))) "pr99863.c":14:12 8 {*cmpdi_ccno_1}
> (expr_list:REG_DEAD (reg/v:DI 203 [ y ])
> (nil)))
> (insn 7 6 8 2 (set (reg:QI 205)
> (eq:QI (reg:CCZ 17 flags)
> (const_int 0 [0]))) "pr99863.c":14:12 802 {*setcc_qi}
> (nil))
> ...
> (insn 40 38 41 2 (set (reg:QI 220)
> (ne:QI (reg:CCZ 17 flags)
> (const_int 0 [0]))) "pr99863.c":15:11 802 {*setcc_qi}
> (expr_list:REG_DEAD (reg:CCZ 17 flags)
> (nil)))
>
> i.e. the problematic REG_DEAD note is gone and I think at this point DF knows
> that CC is live there.
> But replace_read has code to check for live hard regs, see
> note_stores (this_insn, look_for_hardregs, regs_set);
> in there etc.
But find_shift_sequence doesn't seem to check whether the emitted
insns contain any extra hardreg clobbers that make the replacement
invalid.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (10 preceding siblings ...)
2021-04-01 12:45 ` rguenther at suse dot de
@ 2021-04-01 13:01 ` jakub at gcc dot gnu.org
2021-04-01 13:23 ` jakub at gcc dot gnu.org
` (11 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-01 13:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ah, I see the actual problem. replace_read checks the hard regs in the insn
sequence and compares them to the live hard regs at the point of the read_insn,
in the testcase insn 134 after the CC setter 6 and CC users 7 and 40.
CC is not live on the insn 134.
But we emit that sequence elsewhere:
2076 /* Insert this right before the store insn where it will be safe
2077 from later insns that might change it before the read. */
2078 emit_insn_before (insns, store_insn->insn);
and store_insn->insn in this case is insn 19, which is in between insn 7 and
40, and CC is live there.
So, I think we need to do this live hard regs testing in record_store instead
or in addition to.
We already call get_stored_val there just for testing purposes and throw it
away afterwards.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (11 preceding siblings ...)
2021-04-01 13:01 ` jakub at gcc dot gnu.org
@ 2021-04-01 13:23 ` jakub at gcc dot gnu.org
2021-04-01 13:27 ` jakub at gcc dot gnu.org
` (10 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-01 13:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, I think we want to do:
--- gcc/dse.c.jj 2021-01-28 09:59:11.973750676 +0100
+++ gcc/dse.c 2021-04-01 15:16:22.003913061 +0200
@@ -1970,8 +1970,7 @@ get_stored_val (store_info *store_info,
static bool
replace_read (store_info *store_info, insn_info_t store_insn,
- read_info_t read_info, insn_info_t read_insn, rtx *loc,
- bitmap regs_live)
+ read_info_t read_info, insn_info_t read_insn, rtx *loc)
{
machine_mode store_mode = GET_MODE (store_info->mem);
machine_mode read_mode = GET_MODE (read_info->mem);
@@ -2040,7 +2039,8 @@ replace_read (store_info *store_info, in
note_stores (this_insn, look_for_hardregs, regs_set);
}
- bitmap_and_into (regs_set, regs_live);
+ if (store_insn->fixed_regs_live)
+ bitmap_and_into (regs_set, store_insn->fixed_regs_live);
if (!bitmap_empty_p (regs_set))
{
if (dump_file && (dump_flags & TDF_DETAILS))
@@ -2286,7 +2286,7 @@ check_mem_read_rtx (rtx *loc, bb_info_t
offset - store_info->offset,
width)
&& replace_read (store_info, i_ptr, read_info,
- insn_info, loc, bb_info->regs_live))
+ insn_info, loc))
return;
/* The bases are the same, just see if the offsets
@@ -2352,8 +2352,7 @@ check_mem_read_rtx (rtx *loc, bb_info_t
store_info->width)
&& all_positions_needed_p (store_info,
offset - store_info->offset, width)
- && replace_read (store_info, i_ptr, read_info, insn_info, loc,
- bb_info->regs_live))
+ && replace_read (store_info, i_ptr, read_info, insn_info, loc))
return;
remove = canon_true_dependence (store_info->mem,
to fix this. Perhaps together with filling fixed_regs_live on more stores,
currently we fill it in only for insn with exactly one store or on memset
calls, perhaps we want to fill it on insns with more than one store?
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (12 preceding siblings ...)
2021-04-01 13:23 ` jakub at gcc dot gnu.org
@ 2021-04-01 13:27 ` jakub at gcc dot gnu.org
2021-04-01 13:32 ` rguenther at suse dot de
` (9 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-01 13:27 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Though, we don't add the multiple stores case to active_local_stores and all
the replace_read calls are from the active_local_stores chain.
So I think the above patch should DTRT.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (13 preceding siblings ...)
2021-04-01 13:27 ` jakub at gcc dot gnu.org
@ 2021-04-01 13:32 ` rguenther at suse dot de
2021-04-01 13:35 ` jakub at gcc dot gnu.org
` (8 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: rguenther at suse dot de @ 2021-04-01 13:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #15 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 1 Apr 2021, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
>
> --- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Though, we don't add the multiple stores case to active_local_stores and all
> the replace_read calls are from the active_local_stores chain.
> So I think the above patch should DTRT.
I suppose it already fails elsewhere if fixed_regs_live is not
available?
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (14 preceding siblings ...)
2021-04-01 13:32 ` rguenther at suse dot de
@ 2021-04-01 13:35 ` jakub at gcc dot gnu.org
2021-04-01 13:54 ` jakub at gcc dot gnu.org
` (7 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-01 13:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #16 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #15)
> On Thu, 1 Apr 2021, jakub at gcc dot gnu.org wrote:
>
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
> >
> > --- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> > Though, we don't add the multiple stores case to active_local_stores and all
> > the replace_read calls are from the active_local_stores chain.
> > So I think the above patch should DTRT.
>
> I suppose it already fails elsewhere if fixed_regs_live is not
> available?
No. But the way I wrote the above patch, if fixed_regs_live is not available,
it will punt if any hard regs are live in the considered sequence.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (15 preceding siblings ...)
2021-04-01 13:35 ` jakub at gcc dot gnu.org
@ 2021-04-01 13:54 ` jakub at gcc dot gnu.org
2021-04-03 8:07 ` cvs-commit at gcc dot gnu.org
` (6 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-01 13:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
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 #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50496
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50496&action=edit
gcc11-pr99863.patch
Full untested patch I'm going to bootstrap/regtest overnight.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (16 preceding siblings ...)
2021-04-01 13:54 ` jakub at gcc dot gnu.org
@ 2021-04-03 8:07 ` cvs-commit at gcc dot gnu.org
2021-04-03 8:16 ` [Bug rtl-optimization/99863] [10 " jakub at gcc dot gnu.org
` (5 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-03 8:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #18 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:9c7473688e78dc41fd4312a983453df195dd7786
commit r11-7970-g9c7473688e78dc41fd4312a983453df195dd7786
Author: Jakub Jelinek <jakub@redhat.com>
Date: Sat Apr 3 10:07:09 2021 +0200
dse: Fix up hard reg conflict checking in replace_read [PR99863]
Since PR37922 fix RTL DSE has hard register conflict checking
in replace_read, so that if the replacement sequence sets (or typically
just
clobbers) some hard register (usually condition codes) we verify that
hard register is not live.
Unfortunately, it compares the hard reg set clobbered/set by the sequence
(regs_set) against the currently live hard register set, but it then
emits the insn sequence not at the current insn position, but before
store_insn->insn.
So, we should not compare against the current live hard register set,
but against the hard register live set at the point of the store insn.
Fortunately, we already have that remembered in
store_insn->fixed_regs_live.
In addition to bootstrapping/regtesting this patch on x86_64-linux and
i686-linux, I've also added statistics gathering and it seems the only
place where we end up rejecting the replace_read is the newly added
testcase (the PR37922 is no longer effective at that) and fixed_regs_live
has been always non-NULL at the if (store_insn->fixed_regs_live) spot.
Rather than having there an assert, I chose to just keep regs_set
as is, which means in that hypothetical case where fixed_regs_live wouldn't
be computed for some store we'd still accept sequences that don't
clobber/set any hard registers and just punt on those that clobber/set
those.
2021-04-03 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/99863
* dse.c (replace_read): Drop regs_live argument. Instead of
regs_live, use store_insn->fixed_regs_live if non-NULL,
otherwise punt if insns sequence clobbers or sets any hard
registers.
* gcc.target/i386/pr99863.c: New test.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (17 preceding siblings ...)
2021-04-03 8:07 ` cvs-commit at gcc dot gnu.org
@ 2021-04-03 8:16 ` jakub at gcc dot gnu.org
2021-04-08 12:02 ` rguenth at gcc dot gnu.org
` (4 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-03 8:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[10/11 Regression] wrong |[10 Regression] wrong code
|code with -O |with -O -fno-tree-forwprop
|-fno-tree-forwprop |-mno-sse2 since
|-mno-sse2 since |r10-7268-g529ea7d9596b26ba
|r10-7268-g529ea7d9596b26ba |
--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (18 preceding siblings ...)
2021-04-03 8:16 ` [Bug rtl-optimization/99863] [10 " jakub at gcc dot gnu.org
@ 2021-04-08 12:02 ` rguenth at gcc dot gnu.org
2021-04-20 9:45 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-04-08 12:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|10.3 |10.4
--- Comment #20 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10.3 is being released, retargeting bugs to GCC 10.4.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (19 preceding siblings ...)
2021-04-08 12:02 ` rguenth at gcc dot gnu.org
@ 2021-04-20 9:45 ` cvs-commit at gcc dot gnu.org
2021-04-20 9:50 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
23 siblings, 0 replies; 25+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-20 9:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:7a2f91d413eb7a3eb0ba52c7ac9618a35addd12a
commit r10-9721-g7a2f91d413eb7a3eb0ba52c7ac9618a35addd12a
Author: Jakub Jelinek <jakub@redhat.com>
Date: Sat Apr 3 10:07:09 2021 +0200
dse: Fix up hard reg conflict checking in replace_read [PR99863]
Since PR37922 fix RTL DSE has hard register conflict checking
in replace_read, so that if the replacement sequence sets (or typically
just
clobbers) some hard register (usually condition codes) we verify that
hard register is not live.
Unfortunately, it compares the hard reg set clobbered/set by the sequence
(regs_set) against the currently live hard register set, but it then
emits the insn sequence not at the current insn position, but before
store_insn->insn.
So, we should not compare against the current live hard register set,
but against the hard register live set at the point of the store insn.
Fortunately, we already have that remembered in
store_insn->fixed_regs_live.
In addition to bootstrapping/regtesting this patch on x86_64-linux and
i686-linux, I've also added statistics gathering and it seems the only
place where we end up rejecting the replace_read is the newly added
testcase (the PR37922 is no longer effective at that) and fixed_regs_live
has been always non-NULL at the if (store_insn->fixed_regs_live) spot.
Rather than having there an assert, I chose to just keep regs_set
as is, which means in that hypothetical case where fixed_regs_live wouldn't
be computed for some store we'd still accept sequences that don't
clobber/set any hard registers and just punt on those that clobber/set
those.
2021-04-03 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/99863
* dse.c (replace_read): Drop regs_live argument. Instead of
regs_live, use store_insn->fixed_regs_live if non-NULL,
otherwise punt if insns sequence clobbers or sets any hard
registers.
* gcc.target/i386/pr99863.c: New test.
(cherry picked from commit 9c7473688e78dc41fd4312a983453df195dd7786)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (20 preceding siblings ...)
2021-04-20 9:45 ` cvs-commit at gcc dot gnu.org
@ 2021-04-20 9:50 ` jakub at gcc dot gnu.org
2021-04-20 23:34 ` cvs-commit at gcc dot gnu.org
2021-04-22 16:52 ` cvs-commit at gcc dot gnu.org
23 siblings, 0 replies; 25+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-04-20 9:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (21 preceding siblings ...)
2021-04-20 9:50 ` jakub at gcc dot gnu.org
@ 2021-04-20 23:34 ` cvs-commit at gcc dot gnu.org
2021-04-22 16:52 ` cvs-commit at gcc dot gnu.org
23 siblings, 0 replies; 25+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-20 23:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #23 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:e9d56fab891afa3d40ed2ad75264c09aee65ab88
commit r9-9441-ge9d56fab891afa3d40ed2ad75264c09aee65ab88
Author: Jakub Jelinek <jakub@redhat.com>
Date: Sat Apr 3 10:07:09 2021 +0200
dse: Fix up hard reg conflict checking in replace_read [PR99863]
Since PR37922 fix RTL DSE has hard register conflict checking
in replace_read, so that if the replacement sequence sets (or typically
just
clobbers) some hard register (usually condition codes) we verify that
hard register is not live.
Unfortunately, it compares the hard reg set clobbered/set by the sequence
(regs_set) against the currently live hard register set, but it then
emits the insn sequence not at the current insn position, but before
store_insn->insn.
So, we should not compare against the current live hard register set,
but against the hard register live set at the point of the store insn.
Fortunately, we already have that remembered in
store_insn->fixed_regs_live.
In addition to bootstrapping/regtesting this patch on x86_64-linux and
i686-linux, I've also added statistics gathering and it seems the only
place where we end up rejecting the replace_read is the newly added
testcase (the PR37922 is no longer effective at that) and fixed_regs_live
has been always non-NULL at the if (store_insn->fixed_regs_live) spot.
Rather than having there an assert, I chose to just keep regs_set
as is, which means in that hypothetical case where fixed_regs_live wouldn't
be computed for some store we'd still accept sequences that don't
clobber/set any hard registers and just punt on those that clobber/set
those.
2021-04-03 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/99863
* dse.c (replace_read): Drop regs_live argument. Instead of
regs_live, use store_insn->fixed_regs_live if non-NULL,
otherwise punt if insns sequence clobbers or sets any hard
registers.
* gcc.target/i386/pr99863.c: New test.
(cherry picked from commit 7a2f91d413eb7a3eb0ba52c7ac9618a35addd12a)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Bug rtl-optimization/99863] [10 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
` (22 preceding siblings ...)
2021-04-20 23:34 ` cvs-commit at gcc dot gnu.org
@ 2021-04-22 16:52 ` cvs-commit at gcc dot gnu.org
23 siblings, 0 replies; 25+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-22 16:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99863
--- Comment #24 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:e1ef85b4f29e04e97f2ac0998bfd62bac50428b9
commit r8-10904-ge1ef85b4f29e04e97f2ac0998bfd62bac50428b9
Author: Jakub Jelinek <jakub@redhat.com>
Date: Sat Apr 3 10:07:09 2021 +0200
dse: Fix up hard reg conflict checking in replace_read [PR99863]
Since PR37922 fix RTL DSE has hard register conflict checking
in replace_read, so that if the replacement sequence sets (or typically
just
clobbers) some hard register (usually condition codes) we verify that
hard register is not live.
Unfortunately, it compares the hard reg set clobbered/set by the sequence
(regs_set) against the currently live hard register set, but it then
emits the insn sequence not at the current insn position, but before
store_insn->insn.
So, we should not compare against the current live hard register set,
but against the hard register live set at the point of the store insn.
Fortunately, we already have that remembered in
store_insn->fixed_regs_live.
In addition to bootstrapping/regtesting this patch on x86_64-linux and
i686-linux, I've also added statistics gathering and it seems the only
place where we end up rejecting the replace_read is the newly added
testcase (the PR37922 is no longer effective at that) and fixed_regs_live
has been always non-NULL at the if (store_insn->fixed_regs_live) spot.
Rather than having there an assert, I chose to just keep regs_set
as is, which means in that hypothetical case where fixed_regs_live wouldn't
be computed for some store we'd still accept sequences that don't
clobber/set any hard registers and just punt on those that clobber/set
those.
2021-04-03 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/99863
* dse.c (replace_read): Drop regs_live argument. Instead of
regs_live, use store_insn->fixed_regs_live if non-NULL,
otherwise punt if insns sequence clobbers or sets any hard
registers.
* gcc.target/i386/pr99863.c: New test.
(cherry picked from commit 7a2f91d413eb7a3eb0ba52c7ac9618a35addd12a)
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2021-04-22 16:52 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-01 8:44 [Bug target/99863] New: [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 zsojka at seznam dot cz
2021-04-01 9:01 ` [Bug target/99863] " rguenth at gcc dot gnu.org
2021-04-01 9:14 ` rguenth at gcc dot gnu.org
2021-04-01 9:58 ` rguenth at gcc dot gnu.org
2021-04-01 10:02 ` [Bug target/99863] [10/11 Regression] wrong code with -O -fno-tree-forwprop -mno-sse2 since r10-7268-g529ea7d9596b26ba marxin at gcc dot gnu.org
2021-04-01 10:24 ` rguenth at gcc dot gnu.org
2021-04-01 10:45 ` [Bug rtl-optimization/99863] " rguenth at gcc dot gnu.org
2021-04-01 10:47 ` cvs-commit at gcc dot gnu.org
2021-04-01 11:16 ` ebotcazou at gcc dot gnu.org
2021-04-01 11:28 ` ebotcazou at gcc dot gnu.org
2021-04-01 12:26 ` jakub at gcc dot gnu.org
2021-04-01 12:45 ` rguenther at suse dot de
2021-04-01 13:01 ` jakub at gcc dot gnu.org
2021-04-01 13:23 ` jakub at gcc dot gnu.org
2021-04-01 13:27 ` jakub at gcc dot gnu.org
2021-04-01 13:32 ` rguenther at suse dot de
2021-04-01 13:35 ` jakub at gcc dot gnu.org
2021-04-01 13:54 ` jakub at gcc dot gnu.org
2021-04-03 8:07 ` cvs-commit at gcc dot gnu.org
2021-04-03 8:16 ` [Bug rtl-optimization/99863] [10 " jakub at gcc dot gnu.org
2021-04-08 12:02 ` rguenth at gcc dot gnu.org
2021-04-20 9:45 ` cvs-commit at gcc dot gnu.org
2021-04-20 9:50 ` jakub at gcc dot gnu.org
2021-04-20 23:34 ` cvs-commit at gcc dot gnu.org
2021-04-22 16:52 ` 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).