Re: [PATCH v3 5/6] tcg/riscv64: Fix logical vector immediate matching and emission

[Richard Henderson <richard.henderson@linaro.org>]

On 10/29/26 07:20, Richard Henderson wrote:
> On 10/28/26 09:33, Philippe Mathieu-Daudé wrote:
>> and/or/xor_vec always have VECE 0, yet their constant was matched
>> as a byte element and applied at the stale SEW of the previous op:
>> "bic v4.8h, #0xf0" became "vand.vi ..., 15".
>>
>> Add the 'N' constraint: use the smallest element size at which the
>> constant replicates, if that element fits simm5, and switch SEW to
>> it. A wider element can't fit when the smallest doesn't: its upper
>> half repeats the smaller element, which would then be 0 or -1.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: 5a63f5998791 ("tcg/riscv: Add support for basic vector opcodes")
>> Reported-by: Aurelien Jarno <aurelien@aurel32.net>
>> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
>> ---
>>   tcg/riscv64/tcg-target-con-set.h |  1 +
>>   tcg/riscv64/tcg-target-con-str.h |  1 +
>>   tcg/riscv64/tcg-target.c.inc     | 53 +++++++++++++++++++++++++++-----
>>   3 files changed, 48 insertions(+), 7 deletions(-)
>>
>> diff --git a/tcg/riscv64/tcg-target-con-set.h b/tcg/riscv64/tcg- 
>> target-con-set.h
>> index 0fc26d3f98b4..9b4e219fac16 100644
>> --- a/tcg/riscv64/tcg-target-con-set.h
>> +++ b/tcg/riscv64/tcg-target-con-set.h
>> @@ -26,4 +26,5 @@ C_O1_I2(v, v, v)
>>   C_O1_I2(v, vK, v)
>>   C_O1_I2(v, v, vK)
>>   C_O1_I2(v, v, vL)
>> +C_O1_I2(v, v, vN)
>>   C_O1_I4(v, v, vL, vK, vK)
>> diff --git a/tcg/riscv64/tcg-target-con-str.h b/tcg/riscv64/tcg- 
>> target-con-str.h
>> index c04e15ddfa3c..d7e8b2ce4f2f 100644
>> --- a/tcg/riscv64/tcg-target-con-str.h
>> +++ b/tcg/riscv64/tcg-target-con-str.h
>> @@ -19,3 +19,4 @@ CONST('I', TCG_CT_CONST_S12)
>>   CONST('K', TCG_CT_CONST_S5)
>>   CONST('L', TCG_CT_CONST_CMP_VI)
>>   CONST('M', TCG_CT_CONST_M12)
>> +CONST('N', TCG_CT_CONST_LOGICAL_VI)
>> diff --git a/tcg/riscv64/tcg-target.c.inc b/tcg/riscv64/tcg-target.c.inc
>> index 168932711796..4faf7ff36e07 100644
>> --- a/tcg/riscv64/tcg-target.c.inc
>> +++ b/tcg/riscv64/tcg-target.c.inc
>> @@ -116,6 +116,7 @@ static TCGReg 
>> tcg_target_call_oarg_reg(TCGCallReturnKind kind, int slot)
>>   #define TCG_CT_CONST_M12     0x200
>>   #define TCG_CT_CONST_S5      0x400
>>   #define TCG_CT_CONST_CMP_VI  0x800
>> +#define TCG_CT_CONST_LOGICAL_VI 0x1000
>>   #define ALL_GENERAL_REGS   MAKE_64BIT_MASK(0, 32)
>>   #define ALL_VECTOR_REGS    MAKE_64BIT_MASK(32, 32)
>> @@ -384,6 +385,22 @@ static const struct {
>>       [TCG_COND_GEU] = { OPC_VMSGTU_VI,   1, 16, true  },
>>   };
>> +/*
>> + * Return the element size at which VAL is a replicated simm5,
>> + * or -1 if there is none.  Used for vector logical ops, whose
>> + * VECE is not meaningful.
>> + */
>> +static int vec_logical_immediate_vece(int64_t val)
>> +{
>> +    for (int e = MO_8; e <= MO_64; e++) {
>> +        if (val == dup_const(e, val)) {
>> +            int64_t x = sextract64(val, 0, 8 << e);
>> +            return x >= -0x10 && x <= 0x0f ? e : -1;
>> +        }
>> +    }
>> +    g_assert_not_reached();
>> +}
> 
> This assert can trigger...

Bah.  No it can't.  Because of course val == dup_const(MO_64, val) is 
always true.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~