diff mbox

[rs6000] Restrict reload use of FLOAT_REGS

Message ID 1393632667.3554.84.camel@gnopaine
State New
Headers show

Commit Message

Bill Schmidt March 1, 2014, 12:11 a.m. UTC
Hi,

We've encountered a rare bug that occurs when attempting to reload for
an unaligned store in DImode.  For an unaligned store, using stfd gets
preference over std since stfd doesn't have an alignment restriction and
therefore the "m" constraint matches.  However, when there is not a
register available for the REG to be stored, register elimination can
replace the REG with its REQ_EQUIV.  When this is a PLUS, we end up with
an attempt to compute an integer add into a floating-point register, and
things rapidly go downhill.

We had some internal discussion and determined the best way to fix this
is to avoid ever using FLOAT_REGS for a PLUS in
rs6000_preferred_reload_class, similar to what's currently done to avoid
loading constants into FLOAT_REGS.  Uli Weigand pointed out that this
existing test is actually a bit too strong, as rclass could be ALL_REGS
and this prevents us from using GENERAL_REGS in that case.  So I've
relaxed that test to only look for superclasses of FLOAT_REGS.  (If you
feel this is too risky, I can avoid that change.)

The patch below fixes the one case where we've observed this bug in the
wild (it occurred for a particular snapshot of code for an internal
build that doesn't match any public branch).  Because it's dependent on
register spill, it is very difficult to try to produce a test case that
isn't too fragile, so I haven't tried to add one.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu
(--with-cpu=power8) and powerpc64-unknown-linux-gnu (--with-cpu=power7)
with no regressions.  Is this ok for trunk?

Thanks,
Bill


2014-02-28  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* config/rs6000/rs6000.c (rs6000_preferred_reload_class): Disallow
	PLUS rtx's from reloading into a superset of FLOAT_REGS; relax
	constraint on constants to only prevent them from being reloaded
	into a superset of FLOAT_REGS.

Comments

David Edelsohn March 1, 2014, 4:23 a.m. UTC | #1
On Fri, Feb 28, 2014 at 7:11 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> We've encountered a rare bug that occurs when attempting to reload for
> an unaligned store in DImode.  For an unaligned store, using stfd gets
> preference over std since stfd doesn't have an alignment restriction and
> therefore the "m" constraint matches.  However, when there is not a
> register available for the REG to be stored, register elimination can
> replace the REG with its REQ_EQUIV.  When this is a PLUS, we end up with
> an attempt to compute an integer add into a floating-point register, and
> things rapidly go downhill.
>
> We had some internal discussion and determined the best way to fix this
> is to avoid ever using FLOAT_REGS for a PLUS in
> rs6000_preferred_reload_class, similar to what's currently done to avoid
> loading constants into FLOAT_REGS.  Uli Weigand pointed out that this
> existing test is actually a bit too strong, as rclass could be ALL_REGS
> and this prevents us from using GENERAL_REGS in that case.  So I've
> relaxed that test to only look for superclasses of FLOAT_REGS.  (If you
> feel this is too risky, I can avoid that change.)
>
> The patch below fixes the one case where we've observed this bug in the
> wild (it occurred for a particular snapshot of code for an internal
> build that doesn't match any public branch).  Because it's dependent on
> register spill, it is very difficult to try to produce a test case that
> isn't too fragile, so I haven't tried to add one.
>
> Bootstrapped and tested on powerpc64le-unknown-linux-gnu
> (--with-cpu=power8) and powerpc64-unknown-linux-gnu (--with-cpu=power7)
> with no regressions.  Is this ok for trunk?
>
> Thanks,
> Bill
>
>
> 2014-02-28  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * config/rs6000/rs6000.c (rs6000_preferred_reload_class): Disallow
>         PLUS rtx's from reloading into a superset of FLOAT_REGS; relax
>         constraint on constants to only prevent them from being reloaded
>         into a superset of FLOAT_REGS.

This is okay with me. Uli is the best one to comment if this is the right test.

Thanks, David
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 208207)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -16751,7 +16751,8 @@  rs6000_preferred_reload_class (rtx x, enum reg_cla
       && easy_vector_constant (x, mode))
     return ALTIVEC_REGS;
 
-  if (CONSTANT_P (x) && reg_classes_intersect_p (rclass, FLOAT_REGS))
+  if ((CONSTANT_P (x) || GET_CODE (x) == PLUS)
+      && reg_class_subset_p (FLOAT_REGS, rclass))
     return NO_REGS;
 
   if (GET_MODE_CLASS (mode) == MODE_INT && rclass == NON_SPECIAL_REGS)