Re: [Qemu-devel] [PATCH 17/38] tcg: Add gvec expanders for vector shift by scalar

[David Hildenbrand]

On 23.04.19 21:21, Richard Henderson wrote:
> On 4/23/19 11:58 AM, David Hildenbrand wrote:
>>> +void tcg_gen_gvec_shls(unsigned vece, uint32_t dofs, uint32_t aofs,
>>> +                       TCGv_i32 shift, uint32_t oprsz, uint32_t maxsz);
>>> +void tcg_gen_gvec_shrs(unsigned vece, uint32_t dofs, uint32_t aofs,
>>> +                       TCGv_i32 shift, uint32_t oprsz, uint32_t maxsz);
>>> +void tcg_gen_gvec_sars(unsigned vece, uint32_t dofs, uint32_t aofs,
>>> +                       TCGv_i32 shift, uint32_t oprsz, uint32_t maxsz);
>>
>> I assume all irrelevant bits of the shift have to be masked off by the
>> caller, right?
> 
> Correct, just like for integers.
> 
>>
>> On s390x, I would use it for (one variant of) VECTOR ELEMENT SHIFT like
>> this:
>>
>>
>> +static DisasJumpType op_ves(DisasContext *s, DisasOps *o)
>> +{
>> +    const uint8_t es = get_field(s->fields, m4);
>> +    const uint8_t d2 = get_field(s->fields, d2) &
>> +                       (NUM_VEC_ELEMENT_BITS(es) - 1);
>> +    const uint8_t v1 = get_field(s->fields, v1);
>> +    const uint8_t v3 = get_field(s->fields, v3);
>> +    TCGv_i32 shift;
>> +
>> +    if (es > ES_64) {
>> +        gen_program_exception(s, PGM_SPECIFICATION);
>> +        return DISAS_NORETURN;
>> +    }
>> +
>> +    shift = tcg_temp_new_i32();
>> +    tcg_gen_extrl_i64_i32(shift, o->addr1);
>> +    tcg_gen_andi_i32(shift, shift, NUM_VEC_ELEMENT_BITS(es) - 1);
>> +
>> +    switch (s->fields->op2) {
>> +    case 0x30:
>> +        if (likely(!get_field(s->fields, b2))) {
>> +            gen_gvec_fn_2i(shli, es, v1, v3, d2);
>> +        } else {
>> +            gen_gvec_fn_2s(shls, es, v1, v3, shift);
>> +        }
>> +        break;
>> +    case 0x3a:
>> +        if (likely(!get_field(s->fields, b2))) {
>> +            gen_gvec_fn_2i(sari, es, v1, v3, d2);
>> +        } else {
>> +            gen_gvec_fn_2s(sars, es, v1, v3, shift);
>> +        }
>> +        break;
>> +    case 0x38:
>> +        if (likely(!get_field(s->fields, b2))) {
>> +            gen_gvec_fn_2i(shri, es, v1, v3, d2);
>> +        } else {
>> +            gen_gvec_fn_2s(shrs, es, v1, v3, shift);
>> +        }
>> +        break;
>> +    default:
>> +        g_assert_not_reached();
>> +    }
>> +    tcg_temp_free_i32(shift);
>> +    return DISAS_NEXT;
>> +}
> 
> Looks plausible.  I might have hoisted the b2 == 0 check,
> and avoid the other tcg arithmetic when unused.

Makes sense, will do. Thanks!


-- 

Thanks,

David / dhildenb