Bug 23478 - [3.4 regression] Miscompilation due to reloading of a var that is also used in EH pad
Summary: [3.4 regression] Miscompilation due to reloading of a var that is also used i...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 3.4.5
: P2 critical
Target Milestone: 3.4.5
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on: 23579 24232
Blocks:
  Show dependency treegraph
 
Reported: 2005-08-19 11:08 UTC by Jakub Jelinek
Modified: 2005-10-06 13:28 UTC (History)
4 users (show)

See Also:
Host:
Target: x86_64-linux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-08-19 11:35:06


Attachments
pr23478.C (950 bytes, text/plain)
2005-08-19 11:09 UTC, Jakub Jelinek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2005-08-19 11:08:06 UTC
The following testcase is miscompiled on x86_64-linux at -O2.
Before global alloc/reload, the interesting part of code is:
(reg:DI %rax) = (call _Znwm (1))
(reg:DI 81) = (reg:DI %rax)
...
(reg:DI %rdi) = (reg:DI 81)
(call _ZN2C1C1ERK2C3S2_S2_RPS1_ (%rdi, ...)) - may throw, EH pad .Leh
...
(reg:DI %rdi) = (reg:DI 81)
...
(barrier)
.Leh:
(reg:DI %rdi) = (reg:DI 81)
(call _ZdlPv (%rdi))

Now, as the register preassure is pretty high, there don't appear to be any
free usable call saved registers for pseudo 81, so global alloc assigns pseudo
81 into (reg:DI %r10), which is call clobbered.  Then reload saves it into stack
before the _ZN2C1C1ERK2C3S2_S2_RPS1_ call and restores it from the stack slot
after the call.  But doesn't restore it on the EH path as well.
So we end up with:
(reg:DI %rax) = (call _Znwm (1))
(reg:DI %r10 (81)) = (reg:DI %rax)
...
(reg:DI %rdi) = (reg:DI %r10 (81))
(mem:DI %rsp+24) = (reg:DI %r10 (81))
(call _ZN2C1C1ERK2C3S2_S2_RPS1_ (%rdi, ...)) - may throw, EH pad .Leh
(reg:DI %r10 (81)) = (mem:DI %rsp+24)
...
(reg:DI %rdi) = (reg:DI %r10 (81))
...
(barrier)
.Leh:
(reg:DI %rdi) = (reg:DI %r10 (81))
(call _ZdlPv (%rdi))

As this is a reload bug, it is not reproduceable on != 3.4.x compilers I have
tried, which doesn't mean the bug is present on 3.4.x only though.
Comment 1 Jakub Jelinek 2005-08-19 11:09:04 UTC
Created attachment 9546 [details]
pr23478.C

Testcase.
Comment 2 Serge Belyshev 2005-08-19 11:35:06 UTC
Confirmed.
Comment 3 Jakub Jelinek 2005-08-19 13:36:28 UTC
caller-save.c inserts the restore insns after the can_throw_internal () CALL_INSN
and as the rest of reload excepts fixup_abnormal_edges to fix the mess up.
But, fixup_abnormal_edges only inserts the instructions on the fallthrough
edge, not on the fallthrough edge and the EH edge.
Is the bug in fixup_abnormal_edges then (which ought to insert the insns
on all the edges rather than just one) or is the bug in caller-save.c which
would need to take care of this and inserts the restore instructions not only
after the call insn (awaiting fixup_abnormal_edges moving it to next resp. new
bb), but also to the abnormal edge?
It doesn't seem reload.c nor reload1.c (except fixup_abnormal_edges) bothers with
this at all, so my guess would be that fixup_abnormal_edges needs to be changed,
on the other side it surprises me this didn't cause (reported) problems for
so long.
Comment 4 Richard Henderson 2005-08-19 18:32:43 UTC
I think it's caller-save's bug.

The use of fixup_abnormal_edges in reload and reg-stack is to move output
reloads to the fallthru edge.  Well, the output reloads are not used on
the eh edge, because by definition the output was not generated.
Comment 5 Richard Henderson 2005-08-19 18:34:46 UTC
More, since you cannot insert insns on the abnormal EH edge, the fix to 
caller-save needs to be of the form "don't caller-save this variable".
Comment 6 Jakub Jelinek 2005-08-19 18:51:20 UTC
It can't be inserted just on abnormal critical edges:
gcc_assert (!((e->flags & EDGE_ABNORMAL) && EDGE_CRITICAL_P (e)));
So I guess we could insert it on the EH edges if !EDGE_CRITICAL_P and only
only avoid caller-saving it if EDGE_CRITICAL_P.
Comment 7 Richard Henderson 2005-08-19 19:14:10 UTC
Maybe.  I think you'll find that most of the time these edges *are* critical.
I don't think it's worth bothering to make the distinction.
Comment 8 GCC Commits 2005-08-22 16:59:00 UTC
Subject: Bug 23478

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jakub@gcc.gnu.org	2005-08-22 16:58:50

Modified files:
	gcc            : ChangeLog flow.c global.c regs.h 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/opt: pr23478.C 

Log message:
	PR rtl-optimization/23478
	* regs.h (reg_info): Add throw_calls_crossed.
	(REG_N_THROWING_CALLS_CROSSED): Define.
	* flow.c (allocate_reg_life_data): Initialize
	REG_N_THROWING_CALLS_CROSSED.
	(propagate_one_insn, attempt_auto_inc): Update
	REG_N_THROWING_CALLS_CROSSED.
	* global.c (global_alloc): Don't allocate pseudos across
	calls that may throw.
	
	* g++.dg/opt/pr23478.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.9799&r2=2.9800
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/flow.c.diff?cvsroot=gcc&r1=1.634&r2=1.635
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/global.c.diff?cvsroot=gcc&r1=1.130&r2=1.131
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/regs.h.diff?cvsroot=gcc&r1=1.39&r2=1.40
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.5949&r2=1.5950
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/pr23478.C.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 9 Steve Ellcey 2005-08-29 22:51:52 UTC
The patch for this bug appears to be triggering PR 23548.
Comment 10 GCC Commits 2005-09-01 05:29:13 UTC
Subject: Bug 23478

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jakub@gcc.gnu.org	2005-09-01 05:29:04

Modified files:
	gcc            : ChangeLog global.c local-alloc.c 

Log message:
	PR rtl-optimization/23478
	* local-alloc.c (struct qty): Add n_throwing_calls_crossed field.
	(alloc_qty): Initialize it.
	(update_equiv_regs): Clear REG_N_THROWING_CALLS_CROSSED.
	(combine_regs): Combine also n_throwing_calls_crossed fields.
	(find_free_reg): Don't attempt to caller-save pseudos crossing
	calls that might throw.
	* global.c (struct allocno): Add throwing_calls_crossed field.
	(global_alloc): Revert 2005-08-22 change.  Initialize
	throwing_calls_crossed.
	(find_reg): Don't attempt to caller-save pseudos crossing calls that
	might throw.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.9871&r2=2.9872
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/global.c.diff?cvsroot=gcc&r1=1.131&r2=1.132
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/local-alloc.c.diff?cvsroot=gcc&r1=1.153&r2=1.154

Comment 11 GCC Commits 2005-09-01 06:05:18 UTC
Subject: Bug 23478

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-4_0-branch
Changes by:	jakub@gcc.gnu.org	2005-09-01 06:05:00

Modified files:
	gcc            : ChangeLog regs.h global.c flow.c local-alloc.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/opt: pr23478.C 

Log message:
	PR rtl-optimization/23478
	* regs.h (reg_info): Add throw_calls_crossed.
	(REG_N_THROWING_CALLS_CROSSED): Define.
	* flow.c (allocate_reg_life_data): Initialize
	REG_N_THROWING_CALLS_CROSSED.
	(propagate_one_insn, attempt_auto_inc): Update
	REG_N_THROWING_CALLS_CROSSED.
	* local-alloc.c (struct qty): Add n_throwing_calls_crossed field.
	(alloc_qty): Initialize it.
	(update_equiv_regs): Clear REG_N_THROWING_CALLS_CROSSED.
	(combine_regs): Combine also n_throwing_calls_crossed fields.
	(find_free_reg): Don't attempt to caller-save pseudos crossing
	calls that might throw.
	* global.c (struct allocno): Add throwing_calls_crossed field.
	(global_alloc): Initialize throwing_calls_crossed.
	(find_reg): Don't attempt to caller-save pseudos crossing calls that
	might throw.
	
	* g++.dg/opt/pr23478.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.7592.2.403&r2=2.7592.2.404
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/regs.h.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.38&r2=1.38.12.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/global.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.123&r2=1.123.2.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/flow.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.620&r2=1.620.2.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/local-alloc.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.144&r2=1.144.10.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.5084.2.364&r2=1.5084.2.365
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/pr23478.C.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.2.1

Comment 12 GCC Commits 2005-09-01 20:51:18 UTC
Subject: Bug 23478

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	jakub@gcc.gnu.org	2005-09-01 20:51:09

Modified files:
	gcc            : ChangeLog regs.h local-alloc.c flow.c global.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/opt: pr23478.C 

Log message:
	PR rtl-optimization/23478
	* regs.h (reg_info): Add throw_calls_crossed.
	(REG_N_THROWING_CALLS_CROSSED): Define.
	* flow.c (allocate_reg_life_data): Initialize
	REG_N_THROWING_CALLS_CROSSED.
	(propagate_one_insn, attempt_auto_inc): Update
	REG_N_THROWING_CALLS_CROSSED.
	* local-alloc.c (struct qty): Add n_throwing_calls_crossed field.
	(alloc_qty): Initialize it.
	(update_equiv_regs): Clear REG_N_THROWING_CALLS_CROSSED.
	(combine_regs): Combine also n_throwing_calls_crossed fields.
	(find_free_reg): Don't attempt to caller-save pseudos crossing
	calls that might throw.
	* global.c (struct allocno): Add throwing_calls_crossed field.
	(global_alloc): Initialize throwing_calls_crossed.
	(find_reg): Don't attempt to caller-save pseudos crossing calls that
	might throw.
	
	* g++.dg/opt/pr23478.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.912&r2=2.2326.2.913
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/regs.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.31.4.1&r2=1.31.4.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/local-alloc.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.126.4.1&r2=1.126.4.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/flow.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.572.4.4&r2=1.572.4.5
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/global.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.98&r2=1.98.4.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3389.2.426&r2=1.3389.2.427
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/pr23478.C.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.4.1

Comment 13 Jakub Jelinek 2005-09-01 20:52:58 UTC
Should be fixed on 3.4/4.0/HEAD.