------- Comment #10 from meissner at linux dot vnet dot ibm dot com 2010-02-24
21:01 -------
Subject: Re: [4.5 Regression] wrong code for
200.sixtrack with vectorization and -fdata-sections
On Tue, Feb 23, 2010 at 09:57:17PM -0000, bergner at gcc dot gnu dot org wrote:
>
>
> ------- Comment #9 from bergner at gcc dot gnu dot org 2010-02-23 21:57
> -------
> This is a generic reload bug. With this test case and options, we have the
> following instructions just before IRA:
>
> (insn 203 2 213 2 (set (reg/f:DI 256 [ vect_pk.44 ])
> (plus:DI (reg/f:DI 255)
> (const_int 16 [0x10]))) 83 {*adddi3_internal1}
> (expr_list:REG_EQUAL
> (const:DI (plus:DI (symbol_ref:DI ("*.LANCHOR2") [flags 0x182])
> (const_int 16 [0x10])))
> (nil)))
> ...
> (insn 21 214 25 2 pr42431.f:27 (set (reg:DI 270)
> (const_int 16 [0x10])) 371 {*movdi_internal64} (nil))
> ...
> (insn 20 17 23 3 pr42431.f:27 (set (mem:V4SI (reg/f:DI 256 [ vect_pk.44 ]) [4
> S16 A128])
> (reg:V4SI 269)) 926 {*altivec_movv4si} (expr_list:REG_EQUAL
> (const_vector:V4SI [
> (const_int 0 [0x0])
> (const_int 0 [0x0])
> (const_int 0 [0x0])
> (const_int 0 [0x0])
> ])
> (nil)))
>
> (insn 23 20 232 3 pr42431.f:27 (set (mem:V4SI (plus:DI (reg/f:DI 256 [
> vect_pk.44 ])
> (reg:DI 270)) [4 S16 A128])
> (reg:V4SI 269)) 926 {*altivec_movv4si} (expr_list:REG_EQUAL
> (const_vector:V4SI [
> (const_int 0 [0x0])
> (const_int 0 [0x0])
> (const_int 0 [0x0])
> (const_int 0 [0x0])
> ])
> (nil)))
>
> Both r256 and r270 are marked for spilling. This leads to two chained
> reloads:
>
> (const:DI (plus:DI (symbol_ref:DI ("*.LANCHOR2") [flags 0x182]) (const_int
> 16
> [0x10])))
> and
> (const_int 16 [0x10])
>
> With these chained reloads, we eventually drop into
> gen_reload_chain_without_interm_reg_p(). This function does a copy_rtx() to
> create a scratch rtx it can modify to run some tests on and then throw away.
> The bug is that copy_rtx() simply returns the passed in rtx pointer for CONST
> rtxs, so we end up modifying the program's real rtl rather than some scratch
> rtl. Since we cannot execute the copy_rtx/substitue statements without
> causing
> problems, I'm testing the patch below to see if it fixes the
> problem...although, if there are CONSTs buried deep within the reload rtx, we
> probably could still end up with a similar problem on a different test case.
> Comments anyone?
I suspect this may have been the culprit for the bandaid that I put in
rs6000_emit_move (around line 6339 of rs6000.c) to handle const addresses that
were overwritten with a register. Here is the comment that starts the block:
/* Fix up invalid (const (plus (symbol_ref) (reg))) that seems to be created
in the secondary_reload phase, which evidently overwrites the CONST_INT
with a register. */
> Index: reload1.c
> ===================================================================
> --- reload1.c (revision 156816)
> +++ reload1.c (working copy)
> @@ -5221,6 +5221,10 @@
> r1 = r2;
> r2 = n;
> }
> +
> + if (GET_CODE (rld[r1].in) == CONST)
> + return true;
> +
> gcc_assert (reg_mentioned_p (rld[r2].in, rld[r1].in));
> regno = rld[r1].regno >= 0 ? rld[r1].regno : rld[r2].regno;
> gcc_assert (regno >= 0);
>
>
I dunno, would it be better if we cloned copy_rtx to have a varient that
doesn't call shared_const_p in rtl.c and made a full copy? I would think this
would be better than returning true if it is a constant.
If we decide to keep the current code, please put a comment in re-iterating the
reasons mentioned above (and maybe a call to shared_const_p to see if copy_rtx
would copy it).
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42431
