Re: [PATCH] Fix PR63266: Keep track of impact of sign extension in bswap

[Christophe Lyon <christophe.lyon@linaro.org>]

On 25 September 2014 08:39, Thomas Preud'homme
<thomas.preudhomme@arm.com> wrote:
>> From: Christophe Lyon [mailto:christophe.lyon@linaro.org]
>> Sent: Thursday, September 25, 2014 4:28 AM
>
>>
>> Hi Thomas,
>
> Hi Christophe,
>
>>
>> Although I could notice the improvement:
>> Pass disappears           [PASS =>     ]:
>>   gcc.dg/optimize-bswapsi-1.c scan-tree-dump-times bswap "32 bit
>> bswap
>> implementation found at" 3
>> New pass                  [     => PASS]:
>>   gcc.dg/optimize-bswapsi-1.c scan-tree-dump-times bswap "32 bit
>> bswap
>> implementation found at" 4
>>
>> for arm-*, armeb-* and aarch64-* targets, there is no change for
>> aarch64_be: is this expected?
>
> No, but neither is this:
>
> @@ -1905,11 +1913,10 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
>
>             /* Sign extension: result is dependent on the value.  */
>             old_type_size = TYPE_PRECISION (n->type) / BITS_PER_UNIT;
> -           if (!TYPE_UNSIGNED (n->type)
> -               && type_size > old_type_size
> -               && n->n & ((uint64_t) 0xff << ((old_type_size - 1)
> -                                              * BITS_PER_MARKER)))
> -             return NULL;
> +           if (!TYPE_UNSIGNED (n->type) && type_size > old_type_size
> +               && HEAD_MARKER (n->n, old_type_size))
> +             for (i = 0; i < type_size - old_type_size; i++)
> +               n->n |= MARKER_BYTE_UNKNOWN << (type_size - 1 - i);
>
>             if (type_size < 64 / BITS_PER_MARKER)
>               {
>
> type_size - 1 - I gives a number of marker bytes to shift. I forgot to multiply by the number of bits in a marker. Can you do the change locally and tell me if the test now succeed for aarch64_be?
>

While attempting to try this, I noticed that more precisely the test
is currently UNSUPPORTED on aarch64_be,
which is because check_effective_target_bswap only accepts istarget aarch64-*-*.

I didn't try yet to change it into istarget aarch64*-*-*.


> Best regards,
>
> Thomas
>
>
>
>