MIPS: Fix mode mismatch error between Loongson builtin arguments and insn operands.

Matthew Fortune writes:
> 
> That's not what I hoped but is what I was concerned about as I believe it
> means we have a change of behaviour.  It boils down to simply ignoring the
> argument type of unsigned char.  My guess is that a zero extension is
> created but then immediately eliminated because of the paradoxical subreg.
> 
> I think you need to create a temporary and perform the zero extension to
> ensure we honour the unsigned char operand:
> 
>   rtx new_dst = gen_reg_rtx (SImode);
>   emit_insn (gen_zero_extendqisi2 (new_dst, ops[2].value));
>   ops[2].value = foo;
> 
> This should mean that the testcase I sent always has a zero extension but if
> you change the type of 'amount' to be unsigned char then there should not be
> a zero extension as the argument will be assumed to be correctly zero extended
> already and the explicitly introduced zero_extend will be eliminated.
> 

I have made it generate a zero_extend instead of a SUBREG.
However, the pattern associated with gen_zero_extendqisi2 does not work with
immediate operands, so I had to add an extra step in which the argument is put
into a QImode register before being passed to gen_zero_extendqisi2.

Is this OK ?

Regards,
Toma

gcc/

	* config/mips/mips.c (mips_expand_builtin_insn): Convert the QImode
	argument of the pshufh, psllh, psllw, psrah, psraw, psrlh, psrlw
	builtins to SImode and emit a zero-extend, if necessary.

Toma Tabacu <Toma.Tabacu@imgtec.com> writes:
> Matthew Fortune writes:
> >
> > That's not what I hoped but is what I was concerned about as I believe
> > it means we have a change of behaviour.  It boils down to simply
> > ignoring the argument type of unsigned char.  My guess is that a zero
> > extension is created but then immediately eliminated because of the
> paradoxical subreg.
> >
> > I think you need to create a temporary and perform the zero extension
> > to ensure we honour the unsigned char operand:
> >
> >   rtx new_dst = gen_reg_rtx (SImode);
> >   emit_insn (gen_zero_extendqisi2 (new_dst, ops[2].value));
> >   ops[2].value = foo;
> >
> > This should mean that the testcase I sent always has a zero extension
> > but if you change the type of 'amount' to be unsigned char then there
> > should not be a zero extension as the argument will be assumed to be
> > correctly zero extended already and the explicitly introduced
> zero_extend will be eliminated.
> >
> 
> I have made it generate a zero_extend instead of a SUBREG.
> However, the pattern associated with gen_zero_extendqisi2 does not work
> with immediate operands, so I had to add an extra step in which the
> argument is put into a QImode register before being passed to
> gen_zero_extendqisi2.
> 
> Is this OK ?
> 
> Regards,
> Toma
> 
> gcc/
> 
> 	* config/mips/mips.c (mips_expand_builtin_insn): Convert the QImode
> 	argument of the pshufh, psllh, psllw, psrah, psraw, psrlh, psrlw
> 	builtins to SImode and emit a zero-extend, if necessary.
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> da7fa8f..bab5b93 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -16571,9 +16571,35 @@ mips_expand_builtin_insn (enum insn_code icode,
> unsigned int nops,  {
>    machine_mode imode;
>    int rangelo = 0, rangehi = 0, error_opno = 0;
> +  rtx qireg, sireg;
> 
>    switch (icode)
>      {
> +    /* The third operand of these instructions is in SImode, so we need
> to
> +       bring the corresponding builtin argument from QImode into
> SImode.  */
> +    case CODE_FOR_loongson_pshufh:
> +    case CODE_FOR_loongson_psllh:
> +    case CODE_FOR_loongson_psllw:
> +    case CODE_FOR_loongson_psrah:
> +    case CODE_FOR_loongson_psraw:
> +    case CODE_FOR_loongson_psrlh:
> +    case CODE_FOR_loongson_psrlw:
> +      gcc_assert (has_target_p && nops == 3 && ops[2].mode == QImode);
> +      sireg = gen_reg_rtx (SImode);
> +      /* We need to put the immediate in a register because
> +	 gen_zero_extendqisi2 does not accept immediate operands.  */
> +      if (CONST_INT_P (ops[2].value))
> +	{
> +	  qireg = gen_reg_rtx (QImode);
> +	  emit_insn (gen_rtx_SET (qireg, ops[2].value));
> +	  emit_insn (gen_zero_extendqisi2 (sireg, qireg));
> +	} else {
> +	  emit_insn (gen_zero_extendqisi2 (sireg, ops[2].value));
> +	}

Almost but not quite. There is a force_reg helper that takes care of
this i.e. can get rid of the qireg local and the whole if statement.

      emit_insn (gen_zero_extendqisi2 (sireg, force_reg (ops[2].value)));

> +      ops[2].value = sireg;
> +      ops[2].mode = SImode;
> +      break;
> +
>      case CODE_FOR_msa_addvi_b:
>      case CODE_FOR_msa_addvi_h:
>      case CODE_FOR_msa_addvi_w:

OK with that change.

Thanks,
Matthew

Matthew Fortune writes:
> 
> Almost but not quite. There is a force_reg helper that takes care of
> this i.e. can get rid of the qireg local and the whole if statement.
> 
>       emit_insn (gen_zero_extendqisi2 (sireg, force_reg (ops[2].value)));
> 
> OK with that change.
> 
> Thanks,
> Matthew

Committed as r245243.

Thanks,
Toma