diff mbox

[mips] Change the definition of macro SHIFT_COUNT_TRUNCATED

Message ID AANLkTiny-vV=5f7ygLggUy+La+_ebOaDvzMb52M1_N8i@mail.gmail.com
State New
Headers show

Commit Message

Mingjie Xing Sept. 2, 2010, 9:20 a.m. UTC 
2010/9/2 Richard Sandiford <rdsandiford@googlemail.com>:
> It's far from ideal, but I suppose we have to go with it.
> Can you post the complete patch?

Sure. See the attachment.

gcc/ChangeLog

        * config/mips/mips.h (SHIFT_COUNT_TRUNCATED): Change the definition.
        * config/mips/mips.c (mips_shift_truncation_mask): Implement
          TARGET_SHIFT_TRUNCATION_MASK.

gcc/testsuite/ChangeLog

        * gcc.target/mips/loongson-shift-count-truncated-1.c: New.

Is it OK?

Thanks,
Mingjie

Comments

Richard Sandiford Sept. 2, 2010, 6:24 p.m. UTC | #1 
Mingjie Xing <mingjie.xing@gmail.com> writes:
>         * config/mips/mips.h (SHIFT_COUNT_TRUNCATED): Change the definition.
>         * config/mips/mips.c (mips_shift_truncation_mask): Implement
>           TARGET_SHIFT_TRUNCATION_MASK.

I think a more canonical version would be:

	* config/mips/mips.h (SHIFT_COUNT_TRUNCATED): Change the definition.
	* config/mips/mips.c (mips_shift_truncation_mask): New function.
	(TARGET_SHIFT_TRUNCATION_MASK): Define.

> Index: config/mips/mips.c
> ===================================================================
> --- config/mips/mips.c	(revision 163495)
> +++ config/mips/mips.c	(working copy)
> @@ -16335,6 +16335,20 @@ void mips_function_profiler (FILE *file)
>      fprintf (file, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM],
>  	     reg_names[2]);
>  }
> +
> +/* Implement TARGET_SHIFT_TRUNCATION_MASK. We want to keep the current
> +   behaviour of TARGET_SHIFT_TRUNCATION_MASK for non-vector modes even
> +   when TARGET_LOONGSON_2EF is true. */

s/current/default/, since we're writing this for future readers.
Coding style says there should be two spaces after each sentence, so:

/* Implement TARGET_SHIFT_TRUNCATION_MASK.  We want to keep the default
   behaviour of TARGET_SHIFT_TRUNCATION_MASK for non-vector modes even
   when TARGET_LOONGSON_2EF is true.  */

>  /* Define this to be nonzero if shift instructions ignore all but the low-order
>     few bits.  */
> -#define SHIFT_COUNT_TRUNCATED 1
> +#define SHIFT_COUNT_TRUNCATED (TARGET_LOONGSON_2EF ? 0 : 1)

(!TARGET_LOONGSON_2EF)

Please also document why we're doing this.  Let's do that by removing
the current "Define this..." comment -- it's useless boilerplate --
and replacing it with:

/* Standard MIPS integer shifts truncate the shift amount to the
   width of the shifted operand.  However, Loongson vector shifts
   do not truncate the shift amount at all.  */

OK with those changes, thanks.

Richard
diff mbox

Patch

Index: testsuite/gcc.target/mips/loongson-shift-count-truncated-1.c
===================================================================
--- testsuite/gcc.target/mips/loongson-shift-count-truncated-1.c	(revision 0)
+++ testsuite/gcc.target/mips/loongson-shift-count-truncated-1.c	(revision 0)
@@ -0,0 +1,35 @@ 
+/* Test case for SHIFT_COUNT_TRUNCATED on Loongson. */
+
+/* { dg-do run } */
+/* 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.  */
+/* { 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;
+}
Index: config/mips/mips.c
===================================================================
--- config/mips/mips.c	(revision 163495)
+++ config/mips/mips.c	(working copy)
@@ -16335,6 +16335,20 @@  void mips_function_profiler (FILE *file)
     fprintf (file, "\tmove\t%s,%s\n", reg_names[STATIC_CHAIN_REGNUM],
 	     reg_names[2]);
 }
+
+/* Implement TARGET_SHIFT_TRUNCATION_MASK. We want to keep the current
+   behaviour of TARGET_SHIFT_TRUNCATION_MASK for non-vector modes even
+   when TARGET_LOONGSON_2EF is true. */
+
+static unsigned HOST_WIDE_INT
+mips_shift_truncation_mask (enum machine_mode mode)
+{
+  if (TARGET_LOONGSON_2EF && VECTOR_MODE_P (mode))
+    return 0;
+
+  return GET_MODE_BITSIZE (mode) - 1;
+}
+
 
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
@@ -16539,6 +16553,9 @@  void mips_function_profiler (FILE *file)
 #undef TARGET_ASM_OUTPUT_SOURCE_FILENAME
 #define TARGET_ASM_OUTPUT_SOURCE_FILENAME mips_output_filename
 
+#undef TARGET_SHIFT_TRUNCATION_MASK
+#define TARGET_SHIFT_TRUNCATION_MASK mips_shift_truncation_mask
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-mips.h"
Index: config/mips/mips.h
===================================================================
--- config/mips/mips.h	(revision 163495)
+++ config/mips/mips.h	(working copy)
@@ -2446,7 +2446,7 @@  typedef struct mips_args {
 
 /* Define this to be nonzero if shift instructions ignore all but the low-order
    few bits.  */
-#define SHIFT_COUNT_TRUNCATED 1
+#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.  */