| Message ID | 20160211214347.GA22837@ibm-tiger.the-meissners.org |
|---|---|
| State | New |
| Headers | show |
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
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.
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 {