This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Tue, Oct 21, 2014 at 11:28 AM, Thomas Preud'homme
<thomas.preudhomme@arm.com> wrote:
> Hi Richard,
>
> I realized thanks to Christophe Lyon that a shift was not right: the shift count
> is a number of bytes instead of a number of bits.
>
> This extra patch fixes the problem.

Ok.

Thanks,
Richard.

> ChangeLog are as follows:
>
> *** gcc/ChangeLog ***
>
> 2014-09-26  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         * tree-ssa-math-opts.c (find_bswap_or_nop_1): Fix creation of
>         MARKER_BYTE_UNKNOWN markers when handling casts.
>
> *** gcc/testsuite/ChangeLog ***
>
> 2014-10-08  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         * gcc.dg/optimize-bswaphi-1.c: New bswap pass test.
>
> diff --git a/gcc/testsuite/gcc.dg/optimize-bswaphi-1.c b/gcc/testsuite/gcc.dg/optimize-bswaphi-1.c
> index 3e51f04..18aba28 100644
> --- a/gcc/testsuite/gcc.dg/optimize-bswaphi-1.c
> +++ b/gcc/testsuite/gcc.dg/optimize-bswaphi-1.c
> @@ -42,6 +42,20 @@ uint32_t read_be16_3 (unsigned char *data)
>    return *(data + 1) | (*data << 8);
>  }
>
> +typedef int SItype __attribute__ ((mode (SI)));
> +typedef int HItype __attribute__ ((mode (HI)));
> +
> +/* Test that detection of significant sign extension works correctly. This
> +   checks that unknown byte marker are set correctly in cast of cast.  */
> +
> +HItype
> +swap16 (HItype in)
> +{
> +  return (HItype) (((in >> 0) & 0xFF) << 8)
> +               | (((in >> 8) & 0xFF) << 0);
> +}
> +
>  /* { dg-final { scan-tree-dump-times "16 bit load in target endianness found at" 3 "bswap" } } */
> -/* { dg-final { scan-tree-dump-times "16 bit bswap implementation found at" 3 "bswap" { xfail alpha*-*-* arm*-*-* } } } */
> +/* { dg-final { scan-tree-dump-times "16 bit bswap implementation found at" 1 "bswap" { target alpha*-*-* arm*-*-* } } } */
> +/* { dg-final { scan-tree-dump-times "16 bit bswap implementation found at" 4 "bswap" { xfail alpha*-*-* arm*-*-* } } } */
>  /* { dg-final { cleanup-tree-dump "bswap" } } */
> diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c
> index 3c6e935..2ef2333 100644
> --- a/gcc/tree-ssa-math-opts.c
> +++ b/gcc/tree-ssa-math-opts.c
> @@ -1916,7 +1916,8 @@ find_bswap_or_nop_1 (gimple stmt, struct symbolic_number *n, int limit)
>             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);
> +               n->n |= MARKER_BYTE_UNKNOWN
> +                       << ((type_size - 1 - i) * BITS_PER_MARKER);
>
>             if (type_size < 64 / BITS_PER_MARKER)
>               {
>
> regression testsuite run without regression on x86_64-linux-gnu and bswap tests all pass on arm-none-eabi target
>
> Is it ok for trunk?
>
> Best regards,
>
> Thomas
>
>> -----Original Message-----
>> From: Richard Biener [mailto:richard.guenther@gmail.com]
>> Sent: Wednesday, September 24, 2014 4:01 PM
>> To: Thomas Preud'homme
>> Cc: GCC Patches
>> Subject: Re: [PATCH] Fix PR63266: Keep track of impact of sign extension
>> in bswap
>>
>> On Tue, Sep 16, 2014 at 12:24 PM, Thomas Preud'homme
>> <thomas.preudhomme@arm.com> wrote:
>> > Hi all,
>> >
>> > The fix for PR61306 disabled bswap when a sign extension is detected.
>> However this led to a test case regression (and potential performance
>> regression) in case where a sign extension happens but its effect is
>> canceled by other bit manipulation. This patch aims to fix that by having a
>> special marker to track bytes whose value is unpredictable due to sign
>> extension. If the final result of a bit manipulation doesn't contain any
>> such marker then the bswap optimization can proceed.
>>
>> Nice and simple idea.
>>
>> Ok.
>>
>> Thanks,
>> Richard.
>>
>> > *** gcc/ChangeLog ***
>> >
>> > 2014-09-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>> >
>> >         PR tree-optimization/63266
>> >         * tree-ssa-math-opts.c (struct symbolic_number): Add comment
>> about
>> >         marker for unknown byte value.
>> >         (MARKER_MASK): New macro.
>> >         (MARKER_BYTE_UNKNOWN): New macro.
>> >         (HEAD_MARKER): New macro.
>> >         (do_shift_rotate): Mark bytes with unknown values due to sign
>> >         extension when doing an arithmetic right shift. Replace hardcoded
>> >         mask for marker by new MARKER_MASK macro.
>> >         (find_bswap_or_nop_1): Likewise and adjust ORing of two
>> symbolic
>> >         numbers accordingly.
>> >
>> > *** gcc/testsuite/ChangeLog ***
>> >
>> > 2014-09-15  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>> >
>> >         PR tree-optimization/63266
>> >         * gcc.dg/optimize-bswapsi-1.c (swap32_d): New bswap pass test.
>> >
>> >
>> > Testing:
>> >
>> > * Built an arm-none-eabi-gcc cross-compiler and used it to run the
>> testsuite on QEMU emulating Cortex-M3 without any regression
>> > * Bootstrapped on x86_64-linux-gnu target and testsuite was run
>> without regression
>> >
>> >
>> > Ok for trunk?
>
>
>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]