| Message ID | 000001cfff16$50906f00$f1b14d00$@arm.com |
|---|---|
| State | New |
| Headers | show |
On 11/13/2014 08:49 AM, Zhenqiang Chen wrote: > After adding HAVE_cbranchcc4, we can just use HAVE_cbranchcc4. No need to > add a local variable allow_cc_mode. > > Here is the updated patch. This is ok. Since I've already approved Ulrich's s390 fix, there should not be a problem there for long. r~
On Wed, Nov 12, 2014 at 11:49 PM, Zhenqiang Chen <zhenqiang.chen@arm.com> wrote: > >> -----Original Message----- >> From: Richard Henderson [mailto:rth@redhat.com] >> Sent: Thursday, November 06, 2014 4:23 PM >> To: Zhenqiang Chen; 'Jan-Benedict Glaw'; Hartmut Penner; Ulrich Weigand; >> Andreas Krebbel >> Cc: gcc-patches@gcc.gnu.org >> Subject: Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 > build) >> >> On 11/06/2014 08:44 AM, Zhenqiang Chen wrote: >> > Hi, >> > >> > The patch add runtime check to fix s390 build fail >> > (https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00050.html). >> > >> > And there is additional code to workaround s390 cstorecc4 issue. >> > >> > Bootstrap and no make check regression on X86-64. >> > Build s390-linux-gnu and s390x-linux-gnu. >> > >> > I do not have env to run full s390 tests. Would anyone in the TO list >> > help to run the s390 tests? >> > >> > Thanks! >> > -Zhenqiang >> > >> > ChangeLog: >> > 2014-11-06 Zhenqiang Chen <zhenqiang.chen@arm.com> >> > >> > * ifcvt.c (noce_emit_cmove, noce_get_alt_condition, >> > noce_get_condition): >> > Allow CC mode if HAVE_cbranchcc4. >> > (noce_emit_store_flag): Change CC REG as cond_complex. >> > >> > diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index f4002f9..7f7b7c1 100644 >> > --- a/gcc/ifcvt.c >> > +++ b/gcc/ifcvt.c >> > @@ -849,7 +849,10 @@ noce_emit_store_flag (struct noce_if_info >> > *if_info, rtx x, int reversep, >> > enum rtx_code code; >> > >> > cond_complex = (! general_operand (XEXP (cond, 0), VOIDmode) >> > - || ! general_operand (XEXP (cond, 1), VOIDmode)); >> > + || ! general_operand (XEXP (cond, 1), VOIDmode) >> > + /* For s390, CC REG is general_operand. But cstorecc4 >> > only >> > + handles CCZ1, which can not handle others like CCU. >> > */ >> > + || GET_MODE_CLASS (GET_MODE (XEXP (cond, 0))) == >> MODE_CC); >> > >> >> I'd like to know more about this. This seems like a mistake in the > backend. >> >> > +#ifdef HAVE_cbranchcc4 >> > + if (GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC >> > + || cmp_b != const0_rtx >> > + || !(HAVE_cbranchcc4)) >> > +#endif >> >> Please add >> >> #ifndef HAVE_cbranchcc4 >> # define HAVE_cbranchcc4 >> #endif >> >> at the top of ifcvt.c along with all of the others. Then use normal C > tests >> instead of preprocessor tests. > > Thanks for the comment. > >> > + int allow_cc_mode = false; >> > +#ifdef HAVE_cbranchcc4 >> > + allow_cc_mode = HAVE_cbranchcc4; >> > +#endif >> >> E.g. this becomes >> >> bool allow_cc_mode = HAVE_cbranchcc4; > > After adding HAVE_cbranchcc4, we can just use HAVE_cbranchcc4. No need to > add a local variable allow_cc_mode. > > Here is the updated patch. > > diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c > index f4002f9..21f08c2 100644 > --- a/gcc/ifcvt.c > +++ b/gcc/ifcvt.c > @@ -75,6 +75,10 @@ > + 1) > #endif > > +#ifndef HAVE_cbranchcc4 > +#define HAVE_cbranchcc4 0 > +#endif > + > #define IFCVT_MULTIPLE_DUMPS 1 > > #define NULL_BLOCK ((basic_block) NULL) > @@ -1459,10 +1463,16 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx > x, enum rtx_code code, > end_sequence (); > } > > - /* Don't even try if the comparison operands are weird. */ > + /* Don't even try if the comparison operands are weird > + except that the target supports cbranchcc4. */ > if (! general_operand (cmp_a, GET_MODE (cmp_a)) > || ! general_operand (cmp_b, GET_MODE (cmp_b))) > - return NULL_RTX; > + { > + if (!(HAVE_cbranchcc4) > + || GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC > + || cmp_b != const0_rtx) > + return NULL_RTX; > + } > > #if HAVE_conditional_move > unsignedp = (code == LTU || code == GEU > @@ -1909,7 +1919,7 @@ noce_get_alt_condition (struct noce_if_info *if_info, > rtx target, > } > > cond = canonicalize_condition (if_info->jump, cond, reverse, > - earliest, target, false, true); > + earliest, target, HAVE_cbranchcc4, true); > if (! cond || ! reg_mentioned_p (target, cond)) > return NULL; > > @@ -2401,7 +2411,7 @@ noce_get_condition (rtx_insn *jump, rtx_insn > **earliest, bool then_else_reversed > /* Otherwise, fall back on canonicalize_condition to do the dirty > work of manipulating MODE_CC values and COMPARE rtx codes. */ > tmp = canonicalize_condition (jump, cond, reverse, earliest, > - NULL_RTX, false, true); > + NULL_RTX, HAVE_cbranchcc4, true); > > /* We don't handle side-effects in the condition, like handling > REG_INC notes and making sure no duplicate conditions are emitted. */ > > > This caused: FAIL: gcc.dg/torture/fp-int-convert-float128.c -O0 execution test FAIL: gcc.dg/torture/fp-int-convert-float128.c -O1 execution test FAIL: gcc.dg/torture/fp-int-convert-float128.c -O2 execution test FAIL: gcc.dg/torture/fp-int-convert-float128.c -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test FAIL: gcc.dg/torture/fp-int-convert-float128.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test FAIL: gcc.dg/torture/fp-int-convert-float128.c -O3 -fomit-frame-pointer execution test FAIL: gcc.dg/torture/fp-int-convert-float128.c -O3 -g execution test FAIL: gcc.dg/torture/fp-int-convert-float128.c -Os execution test FAIL: gcc.dg/torture/fp-int-convert-float128-timode.c -O0 execution test FAIL: gcc.dg/torture/fp-int-convert-float128-timode.c -O1 execution test FAIL: gcc.dg/torture/fp-int-convert-float128-timode.c -O2 execution test FAIL: gcc.dg/torture/fp-int-convert-float128-timode.c -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test FAIL: gcc.dg/torture/fp-int-convert-float128-timode.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test FAIL: gcc.dg/torture/fp-int-convert-float128-timode.c -O3 -fomit-frame-pointer execution test FAIL: gcc.dg/torture/fp-int-convert-float128-timode.c -O3 -g execution test FAIL: gcc.dg/torture/fp-int-convert-float128-timode.c -Os execution test FAIL: gfortran.dg/erf_3.F90 -O0 execution test FAIL: gfortran.dg/erf_3.F90 -O1 execution test FAIL: gfortran.dg/erf_3.F90 -O2 execution test FAIL: gfortran.dg/erf_3.F90 -O3 -fomit-frame-pointer execution test FAIL: gfortran.dg/erf_3.F90 -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions execution test FAIL: gfortran.dg/erf_3.F90 -O3 -fomit-frame-pointer -funroll-loops execution test FAIL: gfortran.dg/erf_3.F90 -O3 -g execution test FAIL: gfortran.dg/erf_3.F90 -Os execution test on Linux/ia32.
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index f4002f9..21f08c2 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -75,6 +75,10 @@ + 1) #endif +#ifndef HAVE_cbranchcc4 +#define HAVE_cbranchcc4 0 +#endif + #define IFCVT_MULTIPLE_DUMPS 1 #define NULL_BLOCK ((basic_block) NULL) @@ -1459,10 +1463,16 @@ noce_emit_cmove (struct noce_if_info *if_info, rtx x, enum rtx_code code, end_sequence (); } - /* Don't even try if the comparison operands are weird. */ + /* Don't even try if the comparison operands are weird + except that the target supports cbranchcc4. */ if (! general_operand (cmp_a, GET_MODE (cmp_a)) || ! general_operand (cmp_b, GET_MODE (cmp_b))) - return NULL_RTX; + { + if (!(HAVE_cbranchcc4) + || GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC + || cmp_b != const0_rtx) + return NULL_RTX; + } #if HAVE_conditional_move