This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[patch] PR target/36613: reload codegen bug
- From: Michael Matz <matz at suse dot de>
- To: gcc-patches at gcc dot gnu dot org
- Date: Wed, 9 Jul 2008 19:05:00 +0200 (CEST)
- Subject: [patch] PR target/36613: reload codegen bug
Hi,
reload has a problem when it tries to reuse (not to be confused with
inheriting) a reload with a different mode. In the problematic case at
hand we first find a SImode reload of %ecx to %edx (using %ecx as
reload_reg), and try to fit a second reload from %cl to %dl into that.
This is quite fine, just that the merging of both reloads overwrites the
in/out members, and leaves us with a QImode reload. Or better, it merges
the inmode/outmode (depending on size, i.e. leaves the larger), but
overwrites the reload operands itself.
So we are left with a SImode reload dealing with %cl and %dl.
Unfortunately do_input_reload (and do_output_reload) don't care that much
for inmode/outmode (which would be correct here), but rather simply take
what is noted in in/out. So we emit a QImode move, thereby not correctly
emitting the insns for the first reload.
As I noted in the large bugzilla comment I see two ways of fixing this:
1) also take the size of operands into account while merging the in/out
members, in push_reload
2) overhaul do_input_reload and do_output_reload to care for
inmode/outmode _first_ before falling back to the operands.
Option (2) is more complicated as it potentially requires adjusting the
RTXes we find in in/out for the mode we want. There's also a large scary
comment at do_input_reload speculating about all kinds of potential
breakages, it's very old and probably half of the fear in there doesn't
apply anymore, but I didn't feel like poking into 20 year old history to
see why and how it mattered exactly. I think the whole comment is mood
when we rely on inmode/outmode, but in the end I only did (1), found it to
fix the testcase and regstrapped this successfully on i686 and x86_64.
Since that I somewhat changed the patch, so I'm currently regstrapping it
again. Okay for trunk if that passes? At least 4.3 needs that too,
eventually. If someone also wants to test this on 4.2 I would be glad.
Ciao,
Michael.
PR target/36613
* reload.c (push_reload): Merge in,out,in_reg,out_reg members
for reused reload, instead of overwriting them.
* gcc.target/i386/pr36613.c: New testcase.
Index: gcc/reload.c
===================================================================
*** gcc/reload.c (revision 137652)
--- gcc/reload.c (working copy)
*************** push_reload (rtx in, rtx out, rtx *inloc
*** 1403,1415 ****
else
remove_address_replacements (rld[i].in);
}
! rld[i].in = in;
! rld[i].in_reg = in_reg;
}
if (out != 0)
{
! rld[i].out = out;
! rld[i].out_reg = outloc ? *outloc : 0;
}
if (reg_class_subset_p (class, rld[i].class))
rld[i].class = class;
--- 1403,1438 ----
else
remove_address_replacements (rld[i].in);
}
! /* When emitting reloads we don't necessarily look at the in-
! and outmode, but also directly at the operands (in and out).
! So we can't simply overwrite them with whatever we have found
! for this (to-be-merged) reload, we have to "merge" that too.
! Reusing another reload already verified that we deal with the
! same operands, just possibly in different modes. So we
! overwrite the operands only when the new mode is larger.
! See also PR33613. */
! if (!rld[i].in
! || GET_MODE_SIZE (GET_MODE (in))
! > GET_MODE_SIZE (GET_MODE (rld[i].in)))
! rld[i].in = in;
! if (!rld[i].in_reg
! || (in_reg
! && GET_MODE_SIZE (GET_MODE (in_reg))
! > GET_MODE_SIZE (GET_MODE (rld[i].in_reg))))
! rld[i].in_reg = in_reg;
}
if (out != 0)
{
! if (!rld[i].out
! || (out
! && GET_MODE_SIZE (GET_MODE (out))
! > GET_MODE_SIZE (GET_MODE (rld[i].out))))
! rld[i].out = out;
! if (outloc
! && (!rld[i].out_reg
! || GET_MODE_SIZE (GET_MODE (*outloc))
! > GET_MODE_SIZE (GET_MODE (rld[i].out_reg))))
! rld[i].out_reg = *outloc;
}
if (reg_class_subset_p (class, rld[i].class))
rld[i].class = class;