On 04/27/2018 11:04 AM, Alex Bennée wrote:
>
> Richard Henderson <richard.hender...@linaro.org> writes:
>
>> While we have some of the scalar paths for *CVF for fp16,
>> we failed to decode the fp16 version of these instructions.
>>
>> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
>> ---
>> target/arm/translate-a64.c | 33 ++++++++++++++++++++-------------
>> 1 file changed, 20 insertions(+), 13 deletions(-)
>>
>> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
>> index b47319d437..c92e052686 100644
>> --- a/target/arm/translate-a64.c
>> +++ b/target/arm/translate-a64.c
>> @@ -7077,13 +7077,26 @@ static void
>> handle_simd_shift_intfp_conv(DisasContext *s, bool is_scalar,
>> int immh, int immb, int opcode,
>> int rn, int rd)
>> {
>> - bool is_double = extract32(immh, 3, 1);
>> - int size = is_double ? MO_64 : MO_32;
>> - int elements;
>> + int size, elements, fracbits;
>> int immhb = immh << 3 | immb;
>> - int fracbits = (is_double ? 128 : 64) - immhb;
>>
>> - if (!extract32(immh, 2, 2)) {
>> + if (immh & 8) {
>> + size = MO_64;
>> + if (!is_scalar && !is_q) {
>> + unallocated_encoding(s);
>> + return;
>> + }
>> + } else if (immh & 4) {
>> + size = MO_32;
>> + } else if (immh & 2) {
>> + size = MO_16;
>> + if (!arm_dc_feature(s, ARM_FEATURE_V8_FP16)) {
>> + unallocated_encoding(s);
>> + return;
>> + }
>> + } else {
>> + /* immh == 0 would be a failure of the decode logic */
>> + g_assert(immh == 1);
>> unallocated_encoding(s);
>> return;
>> }
>> @@ -7091,20 +7104,14 @@ static void
>> handle_simd_shift_intfp_conv(DisasContext *s, bool is_scalar,
>> if (is_scalar) {
>> elements = 1;
>> } else {
>> - elements = is_double ? 2 : is_q ? 4 : 2;
>> - if (is_double && !is_q) {
>> - unallocated_encoding(s);
>> - return;
>> - }
>> + elements = 8 << is_q >> size;
>
> That is a brain exercise for operator precedence. Would:
>
> elements = (is_q ? 16 : 8) >> size;
>
> be clearer?
I don't think so myself. I thought the double conditional harder to follow.
>> + fracbits = (16 << size) - immhb;
>
> The ship has already sailed on this but I'm wishing we had a
> mosize_to_bits() helper function to be explicit about this
> transformation.
Yeah, that might have been a good thing...
r~
