| Message ID | AANLkTin9Xh=16FET539SBzpj8XU4ySntqtmQxQcax9ux@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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
2010/8/31 Richard Sandiford <rdsandiford@googlemail.com>: > 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. Hi Richard, I made a test on linux kernel. There are 2640 .o files generated by gcc and 419 files have difference in text sections after having SHIFT_COUNT_TRUNCATED set to 0. The strange thing is that when I also defined TARGET_SHIFT_TRUNCATION_MASK, changes still exist for these 419 files. Seems that TARGET_SHIFT_TRUNCATION_MASK doesn't have any effect. Here is the definition, static unsigned HOST_WIDE_INT mips_shift_truncation_mask (enum machine_mode mode) { if (VECTOR_MODE_P (mode)) return 0; return GET_MODE_BITSIZE (mode) - 1; } Regards, Mingjie
Mingjie Xing <mingjie.xing@gmail.com> writes: > 2010/8/31 Richard Sandiford <rdsandiford@googlemail.com>: >> 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. > > Hi Richard, I made a test on linux kernel. There are 2640 .o files > generated by gcc and 419 files have difference in text sections after > having SHIFT_COUNT_TRUNCATED set to 0. Whoa. That's a fair number. > The strange thing is that when I also defined > TARGET_SHIFT_TRUNCATION_MASK, changes still exist for these 419 > files. Seems that TARGET_SHIFT_TRUNCATION_MASK doesn't have any > effect. Probably not too surprising to be honest. T_S_T_M only affects expand, whereas SHIFT_COUNT_TRUNCATED affects the rtl optimisers. It's far from ideal, but I suppose we have to go with it. Can you post the complete patch? Richard
Index: config/mips/mips.h =================================================================== --- config/mips/mips.h (revision 163612) +++ config/mips/mips.h (working copy) @@ -2422,8 +2422,11 @@ typedef struct mips_args { #define SLOW_BYTE_ACCESS (!TARGET_MIPS16) /* Define this to be nonzero if shift instructions ignore all but the low-order - few bits. */ -#define SHIFT_COUNT_TRUNCATED 1 + few bits. + + For Loongson targets, the Loongson specific vector shift instructions are + not SHIFT_COUNT_TRUNCATED. */ +#define SHIFT_COUNT_TRUNCATED (TARGET_LOONGSON_2EF ? 0 : 1) /* Value is 1 if truncating an integer of INPREC bits to OUTPREC bits is done just by pretending it is already truncated. */
Hello, 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? BTW, I'm not sure if I should commit the test case. Thanks, Mingjie