On Tue, 12 May 2020 at 14:09, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Fri, 8 May 2020 at 16:22, Richard Henderson
> <richard.hender...@linaro.org> wrote:
> >
> > Create vectorized versions of handle_shri_with_rndacc
> > for shift+round and shift+round+accumulate. Add out-of-line
> > helpers in preparation for longer vector lengths from SVE.
> >
> > Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> > + /* tszimm encoding produces immediates in the range [1..esize] */
> > + tcg_debug_assert(shift > 0);
>
> This assert can be triggered:
(well, not this assert, but the equivalent one in gen_gvec_ursra)
> > +static void gen_srshr_vec(unsigned vece, TCGv_vec d, TCGv_vec a, int64_t
> > sh)
> > +{
> > + TCGv_vec t = tcg_temp_new_vec_matching(d);
> > + TCGv_vec ones = tcg_temp_new_vec_matching(d);
> > +
> > + tcg_gen_shri_vec(vece, t, a, sh - 1);
> > + tcg_gen_dupi_vec(vece, ones, 1);
> > + tcg_gen_and_vec(vece, t, t, ones);
> > + tcg_gen_sari_vec(vece, d, a, sh);
> > + tcg_gen_add_vec(vece, d, d, t);
> > +
> > + tcg_temp_free_vec(t);
> > + tcg_temp_free_vec(ones);
> > +}
+void gen_gvec_srshr(unsigned vece, uint32_t rd_ofs, uint32_t rm_ofs,
+ int64_t shift, uint32_t opr_sz, uint32_t max_sz)
+{
+ static const TCGOpcode vecop_list[] = {
+ INDEX_op_shri_vec, INDEX_op_sari_vec, INDEX_op_add_vec, 0
+ };
Is there documentation somewhere of which vector operations don't
need to be listed in the vecop list? Here gen_srshr_vec() also
uses 'dupi_vec' and 'and_vec', which aren't listed, presumably
because we guarantee them to be implemented? (Hopefully we don't
encounter a future host vector architecture which breaks that
assumption :-))
> > @@ -5269,6 +5685,28 @@ static int disas_neon_data_insn(DisasContext *s,
> > uint32_t insn)
> > }
> > return 0;
> >
> > + case 2: /* VRSHR */
> > + /* Right shift comes here negative. */
> > + shift = -shift;
> > + if (u) {
> > + gen_gvec_urshr(size, rd_ofs, rm_ofs, shift,
> > + vec_size, vec_size);
> > + } else {
> > + gen_gvec_srshr(size, rd_ofs, rm_ofs, shift,
> > + vec_size, vec_size);
> > + }
> > + return 0;
> > +
> > + case 3: /* VRSRA */
> > + if (u) {
> > + gen_gvec_ursra(size, rd_ofs, rm_ofs, shift,
> > + vec_size, vec_size);
> > + } else {
> > + gen_gvec_srsra(size, rd_ofs, rm_ofs, shift,
> > + vec_size, vec_size);
> > + }
> > + return 0;
>
> I think the VRSRA case needs the same "shift = -shift" as VRSHR.
With this bug fixed,
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
