This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Committed] PR27158: P1 regression in reload for altivec


The following patch resolves PR target/27158 which is an ICE-on-valid
regression on 4.0, 4.1 and mainline compiling altivec code.

The problem concerns reload attempting to materialize illegitimate
constants.  Through most of reload, the code is cautious continually
checking LEGITIMATE_CONSTANT_P for operands and equiv_notes before
placing them in reg_equiv_constant.  This way when find_reloads_toplev
attempts to materialize a register by checking reg_equiv_constant it
is always guaranteed to be legitimate.  However, for SUBREGs of
pseudos with a reg_equiv_constant, we call gen_lowpart or
simplify_gen_subreg on a legitimate constant, without checking whether
the result we get back is a legitimate constant.

In this particular PR, a register is known to constant a vector of
integers, which is legitimate on altivec, but the SUBREG, casts this
to a vector of floats, which isn't, and we eventually run into trouble
attempting to emit a move insn via the rs6000 backend, when we can't
call validate_mem due to reload_in_progress.

The fix is to be more cautious, and check that the results of SUBREGing
a legitimate constant are still legitimate, with the usual target macro
LEGITIMATE_CONSTANT_P.  This is sufficient to resolve the PR, and should
be safe as the rematerialization of SUBREG constants is just an
optimization around the existing SUBREG reload code in the register
allocator.  If we didn't know the SUBREG_REG was equivalent to a
constant, we'd do the same thing.


The following patch has been tested on both powerpc-apple-darwin7.9.0
and i686-pc-linux-gnu with a full "make bootstrap", all default languages
including x86/Ada, and regression tested with a top-level "make -k check"
with no new failures.

I this looks good on mainline, I'll backport it to the 4.0 and 4.1
branches after a few days.  Let me know if there are any problems,
reload changes have a bad reputation for breaking things.

Committed to mainline as revision 113632.



2006-05-08  Roger Sayle  <roger@eyesopen.com>

	PR target/27158
	* reload.c (find_reloads_toplev): Only return the simplified SUBREG
	of a reg_equiv_constant if the result is a legitimate constant.

	* gcc.target/powerpc/pr27158.c: New test case.


Index: reload.c
===================================================================
*** reload.c	(revision 113588)
--- reload.c	(working copy)
*************** find_reloads_toplev (rtx x, int opnum, e
*** 4559,4578 ****
        rtx tem;

        if (subreg_lowpart_p (x)
! 	  && regno >= FIRST_PSEUDO_REGISTER && reg_renumber[regno] < 0
  	  && reg_equiv_constant[regno] != 0
  	  && (tem = gen_lowpart_common (GET_MODE (x),
! 					reg_equiv_constant[regno])) != 0)
  	return tem;

!       if (regno >= FIRST_PSEUDO_REGISTER && reg_renumber[regno] < 0
  	  && reg_equiv_constant[regno] != 0)
  	{
  	  tem =
  	    simplify_gen_subreg (GET_MODE (x), reg_equiv_constant[regno],
  				 GET_MODE (SUBREG_REG (x)), SUBREG_BYTE (x));
  	  gcc_assert (tem);
! 	  return tem;
  	}

        /* If the subreg contains a reg that will be converted to a mem,
--- 4559,4582 ----
        rtx tem;

        if (subreg_lowpart_p (x)
! 	  && regno >= FIRST_PSEUDO_REGISTER
! 	  && reg_renumber[regno] < 0
  	  && reg_equiv_constant[regno] != 0
  	  && (tem = gen_lowpart_common (GET_MODE (x),
! 					reg_equiv_constant[regno])) != 0
! 	  && LEGITIMATE_CONSTANT_P (tem))
  	return tem;

!       if (regno >= FIRST_PSEUDO_REGISTER
! 	  && reg_renumber[regno] < 0
  	  && reg_equiv_constant[regno] != 0)
  	{
  	  tem =
  	    simplify_gen_subreg (GET_MODE (x), reg_equiv_constant[regno],
  				 GET_MODE (SUBREG_REG (x)), SUBREG_BYTE (x));
  	  gcc_assert (tem);
! 	  if (LEGITIMATE_CONSTANT_P (tem))
! 	    return tem;
  	}

        /* If the subreg contains a reg that will be converted to a mem,
*************** find_reloads_toplev (rtx x, int opnum, e
*** 4588,4594 ****
  	 a wider mode if we have a paradoxical SUBREG.  find_reloads will
  	 force a reload in that case.  So we should not do anything here.  */

!       else if (regno >= FIRST_PSEUDO_REGISTER
  #ifdef LOAD_EXTEND_OP
  	       && (GET_MODE_SIZE (GET_MODE (x))
  		   <= GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))))
--- 4592,4598 ----
  	 a wider mode if we have a paradoxical SUBREG.  find_reloads will
  	 force a reload in that case.  So we should not do anything here.  */

!       if (regno >= FIRST_PSEUDO_REGISTER
  #ifdef LOAD_EXTEND_OP
  	       && (GET_MODE_SIZE (GET_MODE (x))
  		   <= GET_MODE_SIZE (GET_MODE (SUBREG_REG (x))))



/* { dg-do compile { target powerpc*-*-* } } */
/* { dg-xfail-if "" { "powerpc-*-eabispe*" "powerpc-ibm-aix*" } { "*" } { "" } } */
/* { dg-options "-O2 -maltivec" } */
#define REGLIST                                                              \
         "77",  "78",  "79",  "80",  "81",  "82",  "83",  "84",  "85",  "86",\
         "87",  "88",  "89",  "90",  "91",  "92",  "93",  "94",  "95",  "96",\
         "97",  "98",  "99", "100", "101", "102", "103", "104", "105", "106",\
        "107", "108"

typedef __attribute__ ((vector_size (16))) float v4sf;

void
foo (int H)
{
  volatile v4sf tmp;
  while (H-- > 0)
    {
      asm ("" : : : REGLIST);
      tmp = (v4sf) __builtin_altivec_vspltisw (1);
    }
}


Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]