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.henderson@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;
}
+ fracbits = (16 << size) - immhb;
if (!fp_access_check(s)) {
return;
}
- /* immh == 0 would be a failure of the decode logic */
- g_assert(immh);
-
handle_simd_intfp_conv(s, rd, rn, elements, !is_u, fracbits, size);
}
--
2.14.3
Richard Henderson <richard.henderson@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.henderson@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? Personally I'd have probably done it long hand in each size
leg above, e.g:
size = MO_16;
elements = is_scalar ? 1 : (is_q ? 8 : 4);
> }
> + 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.
>
> if (!fp_access_check(s)) {
> return;
> }
>
> - /* immh == 0 would be a failure of the decode logic */
> - g_assert(immh);
> -
> handle_simd_intfp_conv(s, rd, rn, elements, !is_u, fracbits, size);
> }
--
Alex Bennée
On 04/27/2018 11:04 AM, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@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.henderson@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~
On 29 April 2018 at 15:44, Richard Henderson <richard.henderson@linaro.org> wrote: > On 04/27/2018 11:04 AM, Alex Bennée wrote: >> >> Richard Henderson <richard.henderson@linaro.org> writes: >>> + 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. Can we at least put in the brackets if we're going to use the << >> expression? thanks -- PMM
© 2016 - 2025 Red Hat, Inc.