public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH], PR 68404 patch #2 (disable power8/power9 fusion on PowerPC)
@ 2016-02-10 22:42 Michael Meissner
  2016-02-10 22:46 ` Jakub Jelinek
  2016-02-11 21:43 ` [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion) Michael Meissner
  0 siblings, 2 replies; 8+ messages in thread
From: Michael Meissner @ 2016-02-10 22:42 UTC (permalink / raw)
  To: gcc-patches, dje.gcc

This patch disables -mcpu=power8/-mtune=power8 from setting -mpower8-fusion and
-mcpu=power9/-mtune=power9 from setting -mpower9-fusion.  I will look at the
earlyclobber that Bernd Schmidt mentioned, but for now it may be safest to just
disable it for GCC 6.0.

I built it on a little endian power8 system, and there were no regressions.  Is
it ok to install?

[gcc]
2016-02-10  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-cpus.def (ISA_2_7_MASKS_SERVER): Do not set
	power8/power9 fusion by default.
	(ISA_3_0_MASKS_SERVER): Likewise.

	* config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
	code setting -mpower8-fusion if -mtune=power8 and -mpower9-fusion
	if -mtune=power9.

	* doc/invoke.texi (RS/6000 and PowerPC Options): Document that
	-mpower8-fusion and -mpower9-fusion are not set by default.

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

	PR target/68404
	* gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
	sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
	* gcc.target/powerpc/fusion2.c: Likewise.
	* gcc.target/powerpc/fusion3.c: Likewise.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH], PR 68404 patch #2 (disable power8/power9 fusion on PowerPC)
  2016-02-10 22:42 [PATCH], PR 68404 patch #2 (disable power8/power9 fusion on PowerPC) Michael Meissner
@ 2016-02-10 22:46 ` Jakub Jelinek
  2016-02-12  0:11   ` David Edelsohn
  2016-02-11 21:43 ` [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion) Michael Meissner
  1 sibling, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2016-02-10 22:46 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, dje.gcc

On Wed, Feb 10, 2016 at 05:42:17PM -0500, Michael Meissner wrote:
> This patch disables -mcpu=power8/-mtune=power8 from setting -mpower8-fusion and
> -mcpu=power9/-mtune=power9 from setting -mpower9-fusion.  I will look at the
> earlyclobber that Bernd Schmidt mentioned, but for now it may be safest to just
> disable it for GCC 6.0.
> 
> I built it on a little endian power8 system, and there were no regressions.  Is
> it ok to install?

Doesn't this mean the bug is still there, just not enabled unless
-mpower[89]-fusion (ok, perhaps mitigated by the previous workaround patch)?
Wouldn't it be better to just forcefully clear the options (and thus ignore
-them) for the time being if they are known to be broken?

> [gcc]
> 2016-02-10  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-cpus.def (ISA_2_7_MASKS_SERVER): Do not set
> 	power8/power9 fusion by default.
> 	(ISA_3_0_MASKS_SERVER): Likewise.
> 
> 	* config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
> 	code setting -mpower8-fusion if -mtune=power8 and -mpower9-fusion
> 	if -mtune=power9.
> 
> 	* doc/invoke.texi (RS/6000 and PowerPC Options): Document that
> 	-mpower8-fusion and -mpower9-fusion are not set by default.
> 
> [gcc/testsuites]
> 2016-02-10  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	PR target/68404
> 	* gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
> 	sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
> 	* gcc.target/powerpc/fusion2.c: Likewise.
> 	* gcc.target/powerpc/fusion3.c: Likewise.
> 
> -- 
> Michael Meissner, IBM
> IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
> email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion)
  2016-02-10 22:42 [PATCH], PR 68404 patch #2 (disable power8/power9 fusion on PowerPC) Michael Meissner
  2016-02-10 22:46 ` Jakub Jelinek
@ 2016-02-11 21:43 ` Michael Meissner
  2016-02-12  0:15   ` David Edelsohn
  2016-02-18 16:45   ` [PATCH], PR 68404 patch #4 " Michael Meissner
  1 sibling, 2 replies; 8+ messages in thread
From: Michael Meissner @ 2016-02-11 21:43 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, dje.gcc

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

After looking at Bernd Schmidt and Jakub Jelinek's suggestions, I came to
conclusion that earlyclobber was not needed in this case, and I removed it.  I
bootstrapped the compiler using profiledbootstrap and lto options and it
succeeded build and running make check.  Just to be sure, I also did a
profiledbootstrap with LTO and -O3 and it built fine.  Is it ok to install
these patches?

I decided to keep the changes to the testsuite explicitly passing the fusion
switches, rather than letting -mtune=power8/power9 set them, but I can be
persuaded to restore the 3 tests to the way they were before February 9th.

[gcc]
2016-02-11  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-11  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/68404
	* gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
	sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
	* gcc.target/powerpc/fusion2.c: Likewise.
	* gcc.target/powerpc/fusion3.c: Likewise.

Since gcc 5.0 also has the earlyclobber in the pattern, I would like to apply
the same change to gcc 5.x (after testing of course), even though we haven't
yet run into the problem with GCC 5.x.  Is this ok as well?

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

[-- Attachment #2: pr68404.patch03b --]
[-- Type: text/plain, Size: 3709 bytes --]

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/fusion2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/fusion2.c	(revision 233351)
+++ gcc/testsuite/gcc.target/powerpc/fusion2.c	(working copy)
@@ -3,7 +3,7 @@
 /* { dg-skip-if "" { powerpc*le-*-* } { "*" } { "" } } */
 /* { 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" } */
+/* { dg-options "-mcpu=power8 -mpower8-fusion -O3" } */
 
 vector double fusion_vector (vector double *p) { return p[2]; }
 
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)
@@ -2,7 +2,7 @@
 /* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
 /* { dg-require-effective-target powerpc_p9vector_ok } */
 /* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power7" } } */
-/* { dg-options "-mcpu=power7 -mtune=power9 -O3" } */
+/* { dg-options "-mcpu=power9 -mpower9-fusion -O3" } */
 
 #define SIZE 4
 struct foo {
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)
@@ -2,7 +2,7 @@
 /* { 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=power8 -mpower8-fusion -O3 -mcmodel=medium" } */
 
 #define SIZE 4
 struct foo {

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH], PR 68404 patch #2 (disable power8/power9 fusion on PowerPC)
  2016-02-10 22:46 ` Jakub Jelinek
@ 2016-02-12  0:11   ` David Edelsohn
  0 siblings, 0 replies; 8+ messages in thread
From: David Edelsohn @ 2016-02-12  0:11 UTC (permalink / raw)
  To: Jakub Jelinek, Michael Meissner; +Cc: GCC Patches

On Wed, Feb 10, 2016 at 2:46 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Feb 10, 2016 at 05:42:17PM -0500, Michael Meissner wrote:
>> This patch disables -mcpu=power8/-mtune=power8 from setting -mpower8-fusion and
>> -mcpu=power9/-mtune=power9 from setting -mpower9-fusion.  I will look at the
>> earlyclobber that Bernd Schmidt mentioned, but for now it may be safest to just
>> disable it for GCC 6.0.
>>
>> I built it on a little endian power8 system, and there were no regressions.  Is
>> it ok to install?
>
> Doesn't this mean the bug is still there, just not enabled unless
> -mpower[89]-fusion (ok, perhaps mitigated by the previous workaround patch)?
> Wouldn't it be better to just forcefully clear the options (and thus ignore
> -them) for the time being if they are known to be broken?
>
>> [gcc]
>> 2016-02-10  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-cpus.def (ISA_2_7_MASKS_SERVER): Do not set
>>       power8/power9 fusion by default.
>>       (ISA_3_0_MASKS_SERVER): Likewise.
>>
>>       * config/rs6000/rs6000.c (rs6000_option_override_internal): Remove
>>       code setting -mpower8-fusion if -mtune=power8 and -mpower9-fusion
>>       if -mtune=power9.
>>
>>       * doc/invoke.texi (RS/6000 and PowerPC Options): Document that
>>       -mpower8-fusion and -mpower9-fusion are not set by default.
>>
>> [gcc/testsuites]
>> 2016-02-10  Michael Meissner  <meissner@linux.vnet.ibm.com>
>>
>>       PR target/68404
>>       * gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
>>       sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
>>       * gcc.target/powerpc/fusion2.c: Likewise.
>>       * gcc.target/powerpc/fusion3.c: Likewise.

Because of the more recent patches that should fix the cause of this
failure, this set of patches now are moot.

- David

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion)
  2016-02-11 21:43 ` [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion) Michael Meissner
@ 2016-02-12  0:15   ` David Edelsohn
  2016-02-12  0:20     ` Michael Meissner
  2016-02-18 16:45   ` [PATCH], PR 68404 patch #4 " Michael Meissner
  1 sibling, 1 reply; 8+ messages in thread
From: David Edelsohn @ 2016-02-12  0:15 UTC (permalink / raw)
  To: Michael Meissner, GCC Patches, David Edelsohn

On Thu, Feb 11, 2016 at 1:43 PM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> After looking at Bernd Schmidt and Jakub Jelinek's suggestions, I came to
> conclusion that earlyclobber was not needed in this case, and I removed it.  I
> bootstrapped the compiler using profiledbootstrap and lto options and it
> succeeded build and running make check.  Just to be sure, I also did a
> profiledbootstrap with LTO and -O3 and it built fine.  Is it ok to install
> these patches?
>
> I decided to keep the changes to the testsuite explicitly passing the fusion
> switches, rather than letting -mtune=power8/power9 set them, but I can be
> persuaded to restore the 3 tests to the way they were before February 9th.
>
> [gcc]
> 2016-02-11  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-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/68404
>         * gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
>         sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
>         * gcc.target/powerpc/fusion2.c: Likewise.
>         * gcc.target/powerpc/fusion3.c: Likewise.
>
> Since gcc 5.0 also has the earlyclobber in the pattern, I would like to apply
> the same change to gcc 5.x (after testing of course), even though we haven't
> yet run into the problem with GCC 5.x.  Is this ok as well?

This is okay for trunk and GCC 5 branch.

Did you test the patch with the first patch reverted?  The first patch
also was correct and fixed a problem, but it also allows this
underlying bug to appear more prominently.  I want to ensure that the
patch was compared with a version of the compiler that elicited the
failure symptoms.

Thanks, David

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion)
  2016-02-12  0:15   ` David Edelsohn
@ 2016-02-12  0:20     ` Michael Meissner
  0 siblings, 0 replies; 8+ messages in thread
From: Michael Meissner @ 2016-02-12  0:20 UTC (permalink / raw)
  To: David Edelsohn; +Cc: Michael Meissner, GCC Patches

On Thu, Feb 11, 2016 at 04:14:59PM -0800, David Edelsohn wrote:
> On Thu, Feb 11, 2016 at 1:43 PM, Michael Meissner
> <meissner@linux.vnet.ibm.com> wrote:
> > After looking at Bernd Schmidt and Jakub Jelinek's suggestions, I came to
> > conclusion that earlyclobber was not needed in this case, and I removed it.  I
> > bootstrapped the compiler using profiledbootstrap and lto options and it
> > succeeded build and running make check.  Just to be sure, I also did a
> > profiledbootstrap with LTO and -O3 and it built fine.  Is it ok to install
> > these patches?
> >
> > I decided to keep the changes to the testsuite explicitly passing the fusion
> > switches, rather than letting -mtune=power8/power9 set them, but I can be
> > persuaded to restore the 3 tests to the way they were before February 9th.
> >
> > [gcc]
> > 2016-02-11  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-11  Michael Meissner  <meissner@linux.vnet.ibm.com>
> >
> >         PR target/68404
> >         * gcc.target/powerpc/fusion.c: Do not assume that -mtune=power8
> >         sets -mpower8-fusion or -mtune=power9 sets -mpower9-fusion.
> >         * gcc.target/powerpc/fusion2.c: Likewise.
> >         * gcc.target/powerpc/fusion3.c: Likewise.
> >
> > Since gcc 5.0 also has the earlyclobber in the pattern, I would like to apply
> > the same change to gcc 5.x (after testing of course), even though we haven't
> > yet run into the problem with GCC 5.x.  Is this ok as well?
> 
> This is okay for trunk and GCC 5 branch.
> 
> Did you test the patch with the first patch reverted?  The first patch
> also was correct and fixed a problem, but it also allows this
> underlying bug to appear more prominently.  I want to ensure that the
> patch was compared with a version of the compiler that elicited the
> failure symptoms.

The patch to predicates.md reverts the original change that I made on February
9th.  I did sync up the trunk to a newer revision, and I can go through a build
without the rs6000.md patch to show that the rs6000.md patch is the one that
fixes the problem if you prefer.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH], PR 68404 patch #4 (fix earlyclobber problem on power8 fusion)
  2016-02-11 21:43 ` [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion) Michael Meissner
  2016-02-12  0:15   ` David Edelsohn
@ 2016-02-18 16:45   ` Michael Meissner
  2016-02-18 18:28     ` David Edelsohn
  1 sibling, 1 reply; 8+ messages in thread
From: Michael Meissner @ 2016-02-18 16:45 UTC (permalink / raw)
  To: Michael Meissner, gcc-patches, dje.gcc

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

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.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797

[-- Attachment #2: pr68404.patch04b --]
[-- Type: text/plain, Size: 5164 bytes --]

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 } } */

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH], PR 68404 patch #4 (fix earlyclobber problem on power8 fusion)
  2016-02-18 16:45   ` [PATCH], PR 68404 patch #4 " Michael Meissner
@ 2016-02-18 18:28     ` David Edelsohn
  0 siblings, 0 replies; 8+ messages in thread
From: David Edelsohn @ 2016-02-18 18:28 UTC (permalink / raw)
  To: Michael Meissner, GCC Patches

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-02-18 18:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-10 22:42 [PATCH], PR 68404 patch #2 (disable power8/power9 fusion on PowerPC) Michael Meissner
2016-02-10 22:46 ` Jakub Jelinek
2016-02-12  0:11   ` David Edelsohn
2016-02-11 21:43 ` [PATCH], PR 68404 patch #3 (fix earlyclobber problem on power8 fusion) Michael Meissner
2016-02-12  0:15   ` David Edelsohn
2016-02-12  0:20     ` Michael Meissner
2016-02-18 16:45   ` [PATCH], PR 68404 patch #4 " Michael Meissner
2016-02-18 18:28     ` David Edelsohn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).