This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [mips] Change the definition of macro SHIFT_COUNT_TRUNCATED
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Mingjie Xing <mingjie dot xing at gmail dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 30 Aug 2010 18:43:52 +0100
- Subject: Re: [mips] Change the definition of macro SHIFT_COUNT_TRUNCATED
- References: <AANLkTin9Xh=16FET539SBzpj8XU4ySntqtmQxQcax9ux@mail.gmail.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