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]

[RS6000] push_secondary_reload ICE


target.h struct secondary_reload_info has two fields that "are for the
use of the backward compatibility hook", default_secondary_reload.
t_icode is used to pass information from one invocation of
default_secondary_reload to the next, between the
targetm.secondary_reload call below in push_secondary_reload to the
same call inside the recursive call to push_secondary_reload a few
lines later.

  rclass = (enum reg_class) targetm.secondary_reload (in_p, x, reload_class,
						      reload_mode, &sri);
  icode = (enum insn_code) sri.icode;

  /* If we don't need any secondary registers, done.  */
  if (rclass == NO_REGS && icode == CODE_FOR_nothing)
    return -1;

  if (rclass != NO_REGS)
    t_reload = push_secondary_reload (in_p, x, opnum, optional, rclass,
				      reload_mode, type, &t_icode, &sri);

If a target secondary_reload hook ever calls default_secondary_reload
on a recursive call to push_secondary_reload, but does not call
default_secondary_reload on the first call, then we have an
uninitialized t_icode field.

The rs6000 backend was doing exactly this on the pr72103 testcase.

This insn
(insn 488 206 406 31 (set (reg:DI 317)
        (unspec:DI [
                (fix:SI (reg:DF 105 28 [orig:173 _34 ] [173]))
            ] UNSPEC_FCTIWZ)) /home/alan/src/tmp/pr72103.ii:47 373 {fctiwz_df}
     (nil))
didn't get a hard reg for reg:DI 317, and ira decided (reasonably)
that reg 317 ought to be in class VSX_REGS.  When storing reg 317 to
its stack slot, the following code in rs6000_secondary_reload
triggered.

  /* If this is a scalar floating point value and we want to load it into the
     traditional Altivec registers, do it via a move via a traditional floating
     point register, unless we have D-form addressing.  Also make sure that
     non-zero constants use a FPR.  */
  if (!done_p && reg_addr[mode].scalar_in_vmx_p
      && !mode_supports_vmx_dform (mode)
      && (rclass == VSX_REGS || rclass == ALTIVEC_REGS)
      && (memory_p || (GET_CODE (x) == CONST_DOUBLE)))
    {
      ret = FLOAT_REGS;
      default_p = false;
      done_p = true;
    }

For the pr72103 testcase this is effectively saying we shouldn't store
a DImode vsx reg to a stack slot directly, but instead should reload
into a float reg.  Which seems a little odd to me..

Anyway, regardless of whether this PR needs more attention, I believe
we should stop using default_secondary_reload or ensure it isn't
presented with uninitialized data.  The following cures the ICE and
has been bootstrapped and regression tested powerpc64le-linux.

OK for mainline?

	PR target/72103
	* config/rs6000/rs6000.c (rs6000_secondary_reload): Initialize
	sri->t_icode.

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 238e845..cea764b 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -19418,6 +19418,7 @@ rs6000_secondary_reload (bool in_p,
 		       && MEM_P (SUBREG_REG (x))));
 
   sri->icode = CODE_FOR_nothing;
+  sri->t_icode = CODE_FOR_nothing;
   sri->extra_cost = 0;
   icode = ((in_p)
 	   ? reg_addr[mode].reload_load

-- 
Alan Modra
Australia Development Lab, IBM


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