Re: [RS6000] push_secondary_reload ICE

[Segher Boessenkool <segher at kernel dot crashing dot org>]

Hi Alan,

Thanks for the writeup.

On Tue, Jul 26, 2016 at 04:09:12PM +0930, Alan Modra wrote:
>   /* 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..

I added SCALAR_FLOAT_MODE_P (mode) to the condition, which also fixes
this particular PR, and makes this do what the comment says it does.
Mike?  What is best here?  Change the comment or change the code?

> 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?

Yes please.  It is also okay for backports (it seems to be needed on 5
and 6, although the testcase in PR72103 does not fail there).

Thanks,


Segher

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