------- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-12
06:58 -------
Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail
On Apr 12, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> On Apr 11, 2005, Josh Conner <[EMAIL PROTECTED]> wrote:
>> I'm now getting a ICE building the mainline (please ignore the
>> "3.4.3" in the directory structure, it's a holdover from the build
>> scripts I'm using) that appears to be related to this patch.
> That sounds possible. The patch might indeed have the effect of
> turning previously-silent miscompilations into ICEs. I didn't expect
> we'd get any, but you may be proving me wrong. Would you please send
> me the preprocessed testcase in private, or just confirm that you're
> using newlib as the C library? I'll then try to duplicate the problem
> on one of the platforms at my disposal.
I went ahead and tried to build an arm-elf toolchain out of uberbaum,
and got the problem.
The problem was an stmia instruction in a loop, whose base address
pseudo loop wanted to replace with another pseudo plus a constant.
Since there's a requirement that all of the base addresses match, when
we started replacing the address expressions of any of the
destinations other than the first, that didn't have an offset, the
insn failed to match, and none of the work arounds I introduced for
such a replacement failure succeeded.
The only relatively simple way to overcome this error I could think of
was to introduce yet another work around, to enable us to handle not
only REGs, but also PLUS involving a REG and a constant (very likely
a literal immediate, but I didn't introduce such a requirement).
This has enabled the testcase to compile successfully and correctly,
although we generate a total of 4 assignments to the pseudo in
response to the 4 givs detected involving it in the 4-register stmia
instruction. Not a big deal, since they end up being optimized away.
The bad news is that this is a regression. The code before my patch
would still work: for some reason I still don't understand, the
assignment of the base pseudo isn't removed, so with my new work
around patch below, we end up introducing unnecessary assignments to
the giv. They are optimized away, but if I can figure out what the
conditions are in which we don't need to introduce assignments to the
giv, we could simply avoid all the messy new code, that assumes we
absolutely must replace the expression with something else.
Anyhow, here's the patch with the workaround for the problem you ran
into. I'll submit it formally when it completes testing.
Index: gcc/loop.c
===================================================================
RCS file: /cvs/uberbaum/gcc/loop.c,v
retrieving revision 1.526
diff -u -p -r1.526 loop.c
--- gcc/loop.c 10 Apr 2005 04:00:45 -0000 1.526
+++ gcc/loop.c 12 Apr 2005 06:35:26 -0000
@@ -5488,6 +5488,15 @@ loop_givs_rescan (struct loop *loop, str
loop_insn_emit_before (loop, 0, v->insn,
gen_move_insn (*v->location,
v->new_reg));
+ else if (GET_CODE (*v->location) == PLUS
+ && REG_P (XEXP (*v->location, 0))
+ && CONSTANT_P (XEXP (*v->location, 1)))
+ loop_insn_emit_before (loop, 0, v->insn,
+ gen_move_insn (XEXP (*v->location, 0),
+ gen_rtx_MINUS
+ (GET_MODE (*v->location),
+ v->new_reg,
+ XEXP (*v->location, 1))));
else
{
/* If it wasn't a reg, create a pseudo and use that. */
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
