RE: [Ping] [PATCH, 6/10] aarch64: add ccmp CC mode

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

> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: Saturday, October 11, 2014 11:32 PM
> To: Zhenqiang Chen; gcc-patches@gcc.gnu.org
> Subject: Re: [Ping] [PATCH, 6/10] aarch64: add ccmp CC mode
> 
> On 09/22/2014 11:44 PM, Zhenqiang Chen wrote:
> > +    case CC_DNEmode:
> > +      return comp_code == NE ? AARCH64_NE : AARCH64_EQ;
> > +    case CC_DEQmode:
> > +      return comp_code == NE ? AARCH64_EQ : AARCH64_NE;
> > +    case CC_DGEmode:
> > +      return comp_code == NE ? AARCH64_GE : AARCH64_LT;
> > +    case CC_DLTmode:
> > +      return comp_code == NE ? AARCH64_LT : AARCH64_GE;
> > +    case CC_DGTmode:
> > +      return comp_code == NE ? AARCH64_GT : AARCH64_LE;
> > +    case CC_DLEmode:
> > +      return comp_code == NE ? AARCH64_LE : AARCH64_GT;
> > +    case CC_DGEUmode:
> > +      return comp_code == NE ? AARCH64_CS : AARCH64_CC;
> > +    case CC_DLTUmode:
> > +      return comp_code == NE ? AARCH64_CC : AARCH64_CS;
> > +    case CC_DGTUmode:
> > +      return comp_code == NE ? AARCH64_HI : AARCH64_LS;
> > +    case CC_DLEUmode:
> > +      return comp_code == NE ? AARCH64_LS : AARCH64_HI;
> 
> I think these should return -1 if comp_code is not EQ.  Like the CC_Zmode
> case below.

Since the code can not guarantee that the CC is used in cbranchcc insns when
expand, it maybe in a tmp register. After some optimizations the CC is
forwarded in cbranchcc insn. So the comp_code might be any legal COMPARE.

And ccmp CC mode has the flag based on NE, so the patch reverts the flag if
not NE.

> Perhaps you can share some code to make the whole thing less bulky.  E.g.
> 
> 	...
> 	case CC_DLEUmode:
> 	  ne = AARCH64_LS;
> 	  eq = AARCH64_HI;
> 	  break;
> 	case CC_Zmode:
> 	  ne = AARCH64_NE;
> 	  eq = AARCH64_EQ;
> 	  break;
>         }
>       if (code == NE)
> 	return ne;
>       if (code == EQ)
> 	return eq;
>       return -1;

Thanks for the comments. Patch is updated.
 
> This does beg the question of whether you need both CC_Zmode and
> CC_DNEmode.
> I'll leave it to an ARM maintainer to say which one of the two should be
kept.
> 
> 
> r~

Attachment: 6-ccmp-cc-mode.patch
Description: Binary data