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

["Zhenqiang Chen" <zhenqiang dot chen at arm dot com>]

> -----Original Message-----
> From: Ulrich Weigand [mailto:uweigand@de.ibm.com]
> Sent: Friday, November 07, 2014 12:11 AM
> To: Richard Henderson
> Cc: Zhenqiang Chen; 'Jan-Benedict Glaw'; Hartmut Penner; Andreas Krebbel;
> gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, ifcvt] Allow CC mode if HAVE_cbranchcc4 (fix s390
build)
> 
> Richard Henderson wrote:
> > 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.
> 
> We do indeed allow the CC register as general_operand, since it has a
> register class of CC_REGS.  This has been in place for over 10 years now
and
> was a deliberate decision to allow for pseudos that carry a condition
value to
> be allocated to the actual CC register or some GPR, depending on reload's
> choices.  As long as moves and (secondary) reloads are fully supported
(and
> they should be), this should be OK ...
> 
> Zhenqiang, what specific problem are you seeing?

Function noce_emit_store_flag tries to generate instruction to store flag by
emit_store_flag for general_operand. For s390, CCU is a general _operand,
but can not match cstorecc4, then it tries to generate a register move
instruction from CCU to CCZ1, which will trigger an ICE.

With the patch (removing the workaround for s390), I got lots of ICE when
building libgcc.

Thank!
-Zhenqiang

> Bye,
> Ulrich
> 
> --
>   Dr. Ulrich Weigand
>   GNU/Linux compilers and toolchain
>   Ulrich.Weigand@de.ibm.com
>