Re: [Qemu-arm] [PATCH v1 03/12] target/arm: Decode aa64 armv8.1 scalar three same extra

[Richard Henderson]

On 11/13/2017 05:30 PM, Alex Bennée wrote:
>>  DEF_HELPER_3(neon_qdmulh_s16, i32, env, i32, i32)
>>  DEF_HELPER_3(neon_qrdmulh_s16, i32, env, i32, i32)
>> +DEF_HELPER_4(neon_qrdmlah_s16, i32, env, i32, i32, i32)
>> +DEF_HELPER_4(neon_qrdmlsh_s16, i32, env, i32, i32, i32)
>>  DEF_HELPER_3(neon_qdmulh_s32, i32, env, i32, i32)
>>  DEF_HELPER_3(neon_qrdmulh_s32, i32, env, i32, i32)
>> +DEF_HELPER_4(neon_qrdmlah_s32, i32, env, s32, s32, s32)
>> +DEF_HELPER_4(neon_qrdmlsh_s32, i32, env, s32, s32, s32)
> 
> Hmm are these really NEON or should they be advsimd_qrdm....?

Same thing.  It seemed nicer to keep the naming consistent.
Probably when we rewrite for SVE we will rename them all...

>> +    switch (opcode) {
>> +    case 0x0: /* SQRDMLAH (vector) */
>> +    case 0x1: /* SQRDMLSH (vector) */
>> +        if (size != 1 && size != 2) {
>> +            unallocated_encoding(s);
>> +            return;
>> +        }
>> +        feature = ARM_FEATURE_V8_1_SIMD;
>> +        break;
>> +    default:
>> +        unallocated_encoding(s);
>> +        return;
>> +    }
>> +
>> +    if (!arm_dc_feature(s, feature)) {
>> +        unallocated_encoding(s);
>> +        return;
>> +    }
> 
> I guess this is because we are expecting additional features to be added
> to the encoding space...

Yes.

>> diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs
>> index 847fb52ee0..c2d32988f9 100644
>> --- a/target/arm/Makefile.objs
>> +++ b/target/arm/Makefile.objs
>> @@ -5,7 +5,7 @@ obj-$(call land,$(CONFIG_KVM),$(call 
>> lnot,$(TARGET_AARCH64))) += kvm32.o
>>  obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o
>>  obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
>>  obj-y += translate.o op_helper.o helper.o cpu.o
>> -obj-y += neon_helper.o iwmmxt_helper.o
>> +obj-y += neon_helper.o iwmmxt_helper.o advsimd_helper.o
> 
> Given this is AArch64 only it makes me wonder if we should be using 
> helper-a64.h?

It isn't aarch64 only.  The aarch32 part is patch 6.


r~