* Re: Bug with copyprop_hardreg_forward and shared RTX
@ 2002-03-04 6:43 Ulrich Weigand
2002-03-04 12:46 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Weigand @ 2002-03-04 6:43 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc
Richard Henderson wrote:
>On Fri, Mar 01, 2002 at 04:58:53PM +0100, Ulrich Weigand wrote:
>> Am I missing something here? How is this supposed to work?
>
>There isn't supposed to be any shared rtx. That's the bug.
However, this appears to be done intentionally by reload,
if the comment at that place is to be believed:
The REG-rtx's for the pseudos are modified in place,
so all insns that used to refer to them now refer to memory.
So if this is the bug, do you have suggestions how to best fix it?
Simply make a pass through all insns and replace the rtx's where
necessary? Or something more involved like having these situations
handled by find_reloads (like the reg_equiv_address case)?
Mit freundlichen Gruessen / Best Regards
Ulrich Weigand
--
Dr. Ulrich Weigand
Linux for S/390 Design & Development
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
Phone: +49-7031/16-3727 --- Email: Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug with copyprop_hardreg_forward and shared RTX
2002-03-04 6:43 Bug with copyprop_hardreg_forward and shared RTX Ulrich Weigand
@ 2002-03-04 12:46 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2002-03-04 12:46 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gcc
On Mon, Mar 04, 2002 at 03:44:43PM +0100, Ulrich Weigand wrote:
> However, this appears to be done intentionally by reload,
> if the comment at that place is to be believed:
>
> The REG-rtx's for the pseudos are modified in place,
> so all insns that used to refer to them now refer to memory.
>
> So if this is the bug, do you have suggestions how to best fix it?
Hum. True, I'd forgotten about that sharing. I'd have thought in
general such substitutions would end up using the stack pointer or
the frame pointer, and so wouldn't end up being optimized.
I guess we could run an unshare_all_rtl_again pass after reload is
done...
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug with copyprop_hardreg_forward and shared RTX
2002-03-06 14:12 Ulrich Weigand
@ 2002-03-06 14:19 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2002-03-06 14:19 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gcc, gcc-patches
On Wed, Mar 06, 2002 at 11:14:39PM +0100, Ulrich Weigand wrote:
> * gcc/reload1.c (reload): Unshare all rtl after reload is done.
Ok for branch and trunk.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug with copyprop_hardreg_forward and shared RTX
@ 2002-03-06 14:12 Ulrich Weigand
2002-03-06 14:19 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Weigand @ 2002-03-06 14:12 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc, gcc-patches
Richard Henderson wrote:
>I guess we could run an unshare_all_rtl_again pass after reload is
>done...
The following patch fixes my test case, and passes bootstrap and
regression tests on s390-ibm-linux and s390x-ibm-linux (done on
the 3.1 branch).
OK to check it in to the branch and the trunk?
ChangeLog:
* gcc/reload1.c (reload): Unshare all rtl after reload is done.
Index: gcc/reload1.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/reload1.c,v
retrieving revision 1.325
diff -c -p -r1.325 reload1.c
*** reload1.c 2002/02/19 02:53:11 1.325
--- reload1.c 2002/03/06 20:23:19
*************** reload (first, global)
*** 1278,1283 ****
--- 1278,1288 ----
unused_insn_chains = 0;
fixup_abnormal_edges ();
+ /* Replacing pseudos with their memory equivalents might have
+ created shared rtx. Subsequent passes would get confused
+ by this, so unshare everything here. */
+ unshare_all_rtl_again (first);
+
return failure;
}
Mit freundlichen Gruessen / Best Regards
Ulrich Weigand
--
Dr. Ulrich Weigand
Linux for S/390 Design & Development
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
Phone: +49-7031/16-3727 --- Email: Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bug with copyprop_hardreg_forward and shared RTX
2002-03-01 7:57 Ulrich Weigand
@ 2002-03-01 9:47 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2002-03-01 9:47 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gcc
On Fri, Mar 01, 2002 at 04:58:53PM +0100, Ulrich Weigand wrote:
> Am I missing something here? How is this supposed to work?
There isn't supposed to be any shared rtx. That's the bug.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Bug with copyprop_hardreg_forward and shared RTX
@ 2002-03-01 7:57 Ulrich Weigand
2002-03-01 9:47 ` Richard Henderson
0 siblings, 1 reply; 6+ messages in thread
From: Ulrich Weigand @ 2002-03-01 7:57 UTC (permalink / raw)
To: rth; +Cc: gcc
Hello Richard,
when running a Linux kernel compiled with gcc 3.1, I've noticed an
intermittent bug with terminal echo handling that I've finally traced
to a routine from the TTY layer being miscompiled.
It looks like the bug is introduced by copyprop_hardreg_forward not
handling shared RTX correctly in certain cases:
Before reload, the code looks like this:
(insn 31 16 46 (set (reg:SI 52)
(mem/s:SI (plus:SI (reg/v/f:SI 42)
(const_int 4 [0x4])) [5 <variable>.c_oflag+0 S4 A32])) 56 {*movsi} (insn_list 8 (nil))
(expr_list:REG_EQUIV (mem/s:SI (plus:SI (reg/v/f:SI 42)
(const_int 4 [0x4])) [5 <variable>.c_oflag+0 S4 A32])
(nil)))
(insn 32 17 47 (parallel[
(set (reg:SI 53)
(xor:SI (reg:SI 52)
(const_int -1 [0xffffffffffffffff])))
(clobber (reg:CC 33 %cc))
] ) 202 {xorsi3} (insn_list 31 (nil))
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil)))
[snip]
(insn 40 35 50 (parallel[
(set (reg:SI 56)
(and:SI (reg:SI 56)
(reg:SI 52)))
(clobber (reg:CC 33 %cc))
] ) 164 {andsi3} (insn_list 37 (insn_list 31 (nil)))
(expr_list:REG_DEAD (reg:SI 52)
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil))))
Note that reg 52 is equivalent to a memory location, and is used twice.
Therefore, after reload, the uses of reg 52 are replaced by a *shared*
copy of this memory RTX:
(gdb) call debug_rtx (cfun->emit->x_regno_reg_rtx[52])
(mem/s:SI (plus:SI (reg/v/f:SI 6 %r6 [42])
(const_int 4 [0x4])) [5 <variable>.c_oflag+0 S4 A32])
and the code looks now like this:
(insn 174 17 32 (set (reg:SI 9 %r9 [53])
(mem/s:SI (plus:SI (reg/v/f:SI 6 %r6 [42])
(const_int 4 [0x4])) [5 <variable>.c_oflag+0 S4 A32])) 56 {*movsi} (nil)
(nil))
(insn 32 174 177 (parallel[
(set (reg:SI 9 %r9 [53])
(xor:SI (reg:SI 9 %r9 [53])
(mem/u/f:SI (symbol_ref/u:SI ("*.LC0")) [5 S4 A32])))
(clobber (reg:CC 33 %cc))
] ) 202 {xorsi3} (insn_list 31 (nil))
(nil))
[snip]
(insn 40 35 50 (parallel[
(set (reg:SI 4 %r4 [56])
(and:SI (reg:SI 4 %r4 [56])
(mem/s:SI (plus:SI (reg/v/f:SI 6 %r6 [42])
(const_int 4 [0x4])) [5 <variable>.c_oflag+0 S4 A32])))
(clobber (reg:CC 33 %cc))
] ) 164 {andsi3} (insn_list 37 (insn_list 31 (nil)))
(nil))
Now copyprop_hardreg_forward decides to replace reg 6 in insn 174
by reg 4, which is completely correct at this point. However, because
the memory RTX is in fact shared between insns 174 and 40, this
replacement gets also done in insn 40, where is it obviously broken
as reg 4 was clobbered in the meantime.
insn 174: replaced reg 6 with 4
(insn 174 17 32 (set (reg:SI 9 %r9 [53])
(mem/s:SI (plus:SI (reg:SI 4 %r4 [42])
(const_int 4 [0x4])) [5 <variable>.c_oflag+0 S4 A32])) 56 {*movsi} (nil)
(nil))
(insn 32 174 177 (parallel[
(set (reg:SI 9 %r9 [53])
(xor:SI (reg:SI 9 %r9 [53])
(mem/u/f:SI (symbol_ref/u:SI ("*.LC0")) [5 S4 A32])))
(clobber (reg:CC 33 %cc))
] ) 202 {xorsi3} (insn_list 174 (nil))
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil)))
[snip]
(insn 40 35 50 (parallel[
(set (reg:SI 4 %r4 [56])
(and:SI (reg:SI 4 %r4 [56])
(mem/s:SI (plus:SI (reg:SI 4 %r4 [42])
(const_int 4 [0x4])) [5 <variable>.c_oflag+0 S4 A32])))
(clobber (reg:CC 33 %cc))
] ) 164 {andsi3} (insn_list 37 (nil))
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil)))
Am I missing something here? How is this supposed to work?
Thanks,
Ulrich
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2002-03-06 22:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-03-04 6:43 Bug with copyprop_hardreg_forward and shared RTX Ulrich Weigand
2002-03-04 12:46 ` Richard Henderson
-- strict thread matches above, loose matches on Subject: below --
2002-03-06 14:12 Ulrich Weigand
2002-03-06 14:19 ` Richard Henderson
2002-03-01 7:57 Ulrich Weigand
2002-03-01 9:47 ` Richard Henderson
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).