diff mbox

, PR 68404 patch #4 (fix earlyclobber problem on power8 fusion)

Message ID 20160218164523.GA19601@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner Feb. 18, 2016, 4:45 p.m. UTC 
This patch to rs6000.md (which is essentially the same as #3) fixes the problem
by removing the early clobber.  The patches to predicates.md, and the fusion
tests revert my changes on February 9th that originally 'solved' the problem by
not allowing fusion of ADDI values.  We have tested the fix using a combine
profiled and LTO bootstrap build and it does not cause any regressions.
Because machine independent changes can mask the bug at times, we also did a
profiled/LTO build on the subversion revision that showed up the problem.  Is
this ok to install in the trunk?

Since gcc 5 contains the exact same early clobber, I would like to also back
port the change to GCC 5.

[gcc]
2016-02-18  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/68404
	* config/rs6000/predicates.md (fusion_gpr_addis): Revert
	2016-02-09 change.

	* config/rs6000/rs6000.md (fusion_gpr_load_<mode>): Remove
	earlyclobber from target.  Use wF constraint for fused memory
	address.
	(fusion_gpr_<P:mode>_<GPR_FUSION:mode>_load): Likewise.

[gcc/testsuites]
2016-02-18  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/68404
	* gcc.target/powerpc/fusion.c: Revert the 2016-02-09 change.
	* gcc.target/powerpc/fusion3.c: Likewise.

Comments

David Edelsohn Feb. 18, 2016, 6:19 p.m. UTC | #1 
On Thu, Feb 18, 2016 at 11:45 AM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> This patch to rs6000.md (which is essentially the same as #3) fixes the problem
> by removing the early clobber.  The patches to predicates.md, and the fusion
> tests revert my changes on February 9th that originally 'solved' the problem by
> not allowing fusion of ADDI values.  We have tested the fix using a combine
> profiled and LTO bootstrap build and it does not cause any regressions.
> Because machine independent changes can mask the bug at times, we also did a
> profiled/LTO build on the subversion revision that showed up the problem.  Is
> this ok to install in the trunk?
>
> Since gcc 5 contains the exact same early clobber, I would like to also back
> port the change to GCC 5.
>
> [gcc]
> 2016-02-18  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/68404
>         * config/rs6000/predicates.md (fusion_gpr_addis): Revert
>         2016-02-09 change.
>
>         * config/rs6000/rs6000.md (fusion_gpr_load_<mode>): Remove
>         earlyclobber from target.  Use wF constraint for fused memory
>         address.
>         (fusion_gpr_<P:mode>_<GPR_FUSION:mode>_load): Likewise.
>
> [gcc/testsuites]
> 2016-02-18  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/68404
>         * gcc.target/powerpc/fusion.c: Revert the 2016-02-09 change.
>         * gcc.target/powerpc/fusion3.c: Likewise.

This is okay for trunk and GCC 5 branch.

Thanks, David
diff mbox

Patch

Index: gcc/config/rs6000/predicates.md
===================================================================
--- gcc/config/rs6000/predicates.md	(revision 233351)
+++ gcc/config/rs6000/predicates.md	(working copy)
@@ -1708,14 +1708,23 @@  (define_predicate "fusion_gpr_addis"
   (match_code "const_int,high,plus")
 {
   HOST_WIDE_INT value;
+  rtx int_const;
 
   if (GET_CODE (op) == HIGH)
     return 1;
 
-  if (!CONST_INT_P (op))
+  if (CONST_INT_P (op))
+    int_const = op;
+
+  else if (GET_CODE (op) == PLUS
+	   && base_reg_operand (XEXP (op, 0), Pmode)
+	   && CONST_INT_P (XEXP (op, 1)))
+    int_const = XEXP (op, 1);
+
+  else
     return 0;
 
-  value = INTVAL (op);
+  value = INTVAL (int_const);
   if ((value & (HOST_WIDE_INT)0xffff) != 0)
     return 0;
 
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 233351)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -12915,8 +12915,8 @@  (define_peephole2
 ;; reload)
 
 (define_insn "fusion_gpr_load_<mode>"
-  [(set (match_operand:INT1 0 "base_reg_operand" "=&b")
-	(unspec:INT1 [(match_operand:INT1 1 "fusion_addis_mem_combo_load" "")]
+  [(set (match_operand:INT1 0 "base_reg_operand" "=b")
+	(unspec:INT1 [(match_operand:INT1 1 "fusion_addis_mem_combo_load" "wF")]
 		     UNSPEC_FUSION_GPR))]
   "TARGET_P8_FUSION"
 {
@@ -12987,7 +12987,7 @@  (define_insn "fusion_gpr_<P:mode>_<GPR_F
 	(unspec:GPR_FUSION
 	 [(match_operand:GPR_FUSION 1 "fusion_addis_mem_combo_load" "wF")]
 	 UNSPEC_FUSION_P9))
-   (clobber (match_operand:P 2 "base_reg_operand" "=&b"))]
+   (clobber (match_operand:P 2 "base_reg_operand" "=b"))]
   "TARGET_P9_FUSION"
 {
   /* This insn is a secondary reload insn, which cannot have alternatives.
Index: gcc/testsuite/gcc.target/powerpc/fusion3.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/fusion3.c	(revision 233351)
+++ gcc/testsuite/gcc.target/powerpc/fusion3.c	(working copy)
@@ -4,24 +4,15 @@ 
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power7" } } */
 /* { dg-options "-mcpu=power7 -mtune=power9 -O3" } */
 
-#define SIZE 4
-struct foo {
-  float f;
-  double d;
-};
+#define LARGE 0x12345
 
-static struct foo st[SIZE];
-struct foo *ptr_st = &st[0];
+int fusion_float_read (float *p){ return p[LARGE]; }
+int fusion_double_read (double *p){ return p[LARGE]; }
 
-float fusion_float_read (void){ return st[SIZE].f; }
-double fusion_float_extend (void){ return (double)st[SIZE].f; }
-double fusion_double_read (void){ return st[SIZE].d; }
+void fusion_float_write (float *p, float f){ p[LARGE] = f; }
+void fusion_double_write (double *p, double d){ p[LARGE] = d; }
 
-void fusion_float_write (float f){ st[SIZE].f = f; }
-void fusion_float_truncate (double d){ st[SIZE].f = (float)d; }
-void fusion_double_write (double d){ st[SIZE].d = d; }
-
-/* { dg-final { scan-assembler-times "load fusion, type SF"  2 } } */
-/* { dg-final { scan-assembler-times "load fusion, type DF"  1 } } */
-/* { dg-final { scan-assembler-times "store fusion, type SF" 2 } } */
-/* { dg-final { scan-assembler-times "store fusion, type DF" 1 } } */
+/* { dg-final { scan-assembler "load fusion, type SF"  } } */
+/* { dg-final { scan-assembler "load fusion, type DF"  } } */
+/* { dg-final { scan-assembler "store fusion, type SF" } } */
+/* { dg-final { scan-assembler "store fusion, type DF" } } */
Index: gcc/testsuite/gcc.target/powerpc/fusion.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/fusion.c	(revision 233351)
+++ gcc/testsuite/gcc.target/powerpc/fusion.c	(working copy)
@@ -1,28 +1,17 @@ 
-/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
+/* { dg-do compile { target { powerpc*-*-* } } } */
 /* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
 /* { dg-require-effective-target powerpc_p8vector_ok } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power7" } } */
-/* { dg-options "-mcpu=power7 -mtune=power8 -O3 -mcmodel=medium" } */
+/* { dg-options "-mcpu=power7 -mtune=power8 -O3" } */
 
-#define SIZE 4
-struct foo {
-  unsigned char uc;
-  signed char sc;
-  unsigned short us;
-  short ss;
-  int i;
-  unsigned u;
-};
+#define LARGE 0x12345
 
-static struct foo st[SIZE];
-struct foo *ptr_st = &st[0];
-
-int fusion_uchar (void){ return st[SIZE-1].uc; }
-int fusion_schar (void){ return st[SIZE-1].sc; }
-int fusion_ushort (void){ return st[SIZE-1].us; }
-int fusion_short (void){ return st[SIZE-1].ss; }
-int fusion_int (void){ return st[SIZE-1].i; }
-unsigned fusion_uns (void){ return st[SIZE-1].u; }
+int fusion_uchar (unsigned char *p){ return p[LARGE]; }
+int fusion_schar (signed char *p){ return p[LARGE]; }
+int fusion_ushort (unsigned short *p){ return p[LARGE]; }
+int fusion_short (short *p){ return p[LARGE]; }
+int fusion_int (int *p){ return p[LARGE]; }
+unsigned fusion_uns (unsigned *p){ return p[LARGE]; }
 
 /* { dg-final { scan-assembler-times "gpr load fusion"    6 } } */
 /* { dg-final { scan-assembler-times "lbz"                2 } } */