Re: [mips] Change the definition of macro SHIFT_COUNT_TRUNCATED

[Richard Sandiford <rdsandiford at googlemail dot com>]

Mingjie Xing <mingjie.xing@gmail.com> writes:
> This patch change the definition of macro SHIFT_COUNT_TRUNCATED. This
> can fix the bug on Loongson for such an test case,
>
> /* { dg-do run } */
> /* { dg-options "isa=loongson -mhard-float -mno-mips16 -O1" } */
>
> #include "loongson.h"
> #include <assert.h>
>
> typedef union { int32x2_t v; int32_t a[2]; } int32x2_encap_t;
>
> void
> main1 (int shift)
> {
>   int32x2_encap_t s;
>   int32x2_encap_t r;
>
>   s.a[0] = 0xffffffff;
>   s.a[1] = 0xffffffff;
>   /* Loongson SIMD use low-order 7 bits to specify the shift amount. Thus
>    V2SI << 0x40 == 0. The below expression 'shift & 0x3f' will be mis-optimized
>    as 'shift', if SHIFT_COUNT_TRUNCATED is nonzero. */
>   r.v = psllw_s (s.v, (shift & 0x3f));
>   assert (r.a[0] == 0xffffffff);
>   assert (r.a[1] == 0xffffffff);
> }
>
> int
> main (void)
> {
>   main1 (0x40);
>   return 0;
> }
>
> ChangeLog:
> 2010-08-28  Mingjie Xing  <mingjie.xing@gmail.com>
>
>         * config/mips/mips.h (SHIFT_COUNT_TRUNCATED) : Define to 0 for
> Loongson targets, 1 otherwise.
>
> Is it OK?

You should also define TARGET_SHIFT_TRUNCATION_MASK.  We want to keep
the current behaviour of TARGET_SHIFT_TRUNCATION_MASK for non-vector
modes even when TARGET_LOONGSON is true.

Would you mind doing a simple check on a "real-life" body of code
to see how many times having SHIFT_COUNT_TRUNCATED set to 0 affects
optimisation of non-vector code?  Something like the linux kernel
would be good.  E.g. see how many linux .o files have different
text sections before and after the patch.

> BTW, I'm not sure if I should commit the test case.

Yeah, when the final patch goes in, please add the testcase too.
What you posted looks good, but please copy the comment:

/* loongson.h does not handle or check for MIPS16ness.  There doesn't
   seem any good reason for it to, given that the Loongson processors
   do not support MIPS16.  */

from loongson-simd.c, and put it above the dg-options line.  The way
we normally force non-MIPS16 mode is to add NOMIPS16 to the function
definition, but the comment is explaining why that doesn't work here.

Richard