Toma Tabacu <toma.tab...@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