Re: [PATCH v4 06/16] tcg/s390x: Implement tcg_out_mov for vector types

From: David Hildenbrand <david@redhat.com>
To: Richard Henderson <richard.henderson@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [PATCH v4 06/16] tcg/s390x: Implement tcg_out_mov for vector types
Date: Tue, 14 Sep 2021 18:56:09 +0200	[thread overview]
Message-ID: <2808db81-0743-e10d-d28e-5b9d25e13ec6@redhat.com> (raw)
In-Reply-To: <045600e5-a3bf-18f3-b8a8-9e593c97a3ed@redhat.com>

On 14.09.21 18:53, David Hildenbrand wrote:
> On 26.06.21 07:02, Richard Henderson wrote:
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>    tcg/s390x/tcg-target.c.inc | 72 +++++++++++++++++++++++++++++++++++---
>>    1 file changed, 68 insertions(+), 4 deletions(-)
>>
>> diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc
>> index b6ea129e14..c4e12a57f3 100644
>> --- a/tcg/s390x/tcg-target.c.inc
>> +++ b/tcg/s390x/tcg-target.c.inc
>> @@ -265,6 +265,11 @@ typedef enum S390Opcode {
>>        RX_STC      = 0x42,
>>        RX_STH      = 0x40,
>>    
>> +    VRRa_VLR    = 0xe756,
>> +
>> +    VRSb_VLVG   = 0xe722,
>> +    VRSc_VLGV   = 0xe721,
>> +
>>        VRX_VL      = 0xe706,
>>        VRX_VLLEZ   = 0xe704,
>>        VRX_VST     = 0xe70e,
>> @@ -548,6 +553,39 @@ static int RXB(TCGReg v1, TCGReg v2, TCGReg v3, TCGReg v4)
>>             | ((v4 & 0x10) << (4 + 0));
>>    }
>>    
>> +static void tcg_out_insn_VRRa(TCGContext *s, S390Opcode op,
>> +                              TCGReg v1, TCGReg v2, int m3)
>> +{
>> +    tcg_debug_assert(v1 >= TCG_REG_V0 && v1 <= TCG_REG_V31);
>> +    tcg_debug_assert(v2 >= TCG_REG_V0 && v2 <= TCG_REG_V31);
>> +    tcg_out16(s, (op & 0xff00) | ((v1 & 15) << 4) | (v2 & 15));
>> +    tcg_out32(s, (op & 0x00ff) | RXB(v1, v2, 0, 0) | (m3 << 12));
>> +}
>> +
>> +static void tcg_out_insn_VRSb(TCGContext *s, S390Opcode op, TCGReg v1,
>> +                              intptr_t d2, TCGReg b2, TCGReg r3, int m4)
>> +{
>> +    tcg_debug_assert(v1 >= TCG_REG_V0 && v1 <= TCG_REG_V31);
>> +    tcg_debug_assert(d2 >= 0 && d2 <= 0xfff);
>> +    tcg_debug_assert(b2 <= TCG_REG_R15);
>> +    tcg_debug_assert(r3 <= TCG_REG_R15);
>> +    tcg_out16(s, (op & 0xff00) | ((v1 & 15) << 4) | r3);
>> +    tcg_out16(s, b2 << 12 | d2);
>> +    tcg_out16(s, (op & 0x00ff) | RXB(v1, 0, 0, 0) | (m4 << 12));
>> +}
>> +
>> +static void tcg_out_insn_VRSc(TCGContext *s, S390Opcode op, TCGReg r1,
>> +                              intptr_t d2, TCGReg b2, TCGReg v3, int m4)
>> +{
>> +    tcg_debug_assert(r1 <= TCG_REG_R15);
>> +    tcg_debug_assert(d2 >= 0 && d2 <= 0xfff);
>> +    tcg_debug_assert(b2 <= TCG_REG_R15);
>> +    tcg_debug_assert(v3 >= TCG_REG_V0 && v3 <= TCG_REG_V31);
>> +    tcg_out16(s, (op & 0xff00) | (r1 << 4) | (v3 & 15));
>> +    tcg_out16(s, b2 << 12 | d2);
>> +    tcg_out16(s, (op & 0x00ff) | RXB(0, 0, v3, 0) | (m4 << 12));
>> +}
>> +
>>    static void tcg_out_insn_VRX(TCGContext *s, S390Opcode op, TCGReg v1,
>>                                 TCGReg b2, TCGReg x2, intptr_t d2, int m3)
>>    {
>> @@ -581,12 +619,38 @@ static void tcg_out_sh32(TCGContext* s, S390Opcode op, TCGReg dest,
>>    
>>    static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg dst, TCGReg src)
>>    {
>> -    if (src != dst) {
>> -        if (type == TCG_TYPE_I32) {
>> +    if (src == dst) {
>> +        return true;
>> +    }
>> +    switch (type) {
>> +    case TCG_TYPE_I32:
>> +        if (likely(dst < 16 && src < 16)) {
>>                tcg_out_insn(s, RR, LR, dst, src);
>> -        } else {
>> -            tcg_out_insn(s, RRE, LGR, dst, src);
>> +            break;
>>            }
>> +        /* fallthru */
>> +
> 
> Does that fall-through work as expected? I would have thought we would
> want to pass "2" as m4 for VLGV and VLVG below?
> 

Forget my question, we're not doing memory access :)

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb