Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390 build)

["H.J. Lu" <hjl dot tools at gmail dot com>]

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.




-- 
H.J.