RE: [Ping] [PATCH, 6/10] aarch64: add ccmp CC mode
-Original Message- From: Richard Henderson [mailto:r...@redhat.com] Sent: Monday, October 27, 2014 11:20 PM To: Zhenqiang Chen Cc: gcc-patches@gcc.gnu.org Subject: Re: [Ping] [PATCH, 6/10] aarch64: add ccmp CC mode On 10/27/2014 12:48 AM, Zhenqiang Chen wrote: -Original Message- From: Richard Henderson [mailto:r...@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. Um, no. The point of returning -1 is to avoid combining with comparisons for which we cannot produce the proper result. Patch is updated. E.g. the existing CC_Zmode, where only the Z bit is valid. We want to reject combination with LTU, which checks the C bit. Are you honestly suggesting that using CC_DNEmode with GE can be made to make sense in any way? No. I misuse the function in previous patch. Now it returns -1. Thanks! -Zhenqiang 6-ccmp-cc-mode.patch Description: Binary data
Re: [Ping] [PATCH, 6/10] aarch64: add ccmp CC mode
On 10/29/2014 03:31 AM, Zhenqiang Chen wrote: Patch is updated. Looks good. r~
RE: [Ping] [PATCH, 6/10] aarch64: add ccmp CC mode
-Original Message- From: Richard Henderson [mailto:r...@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~ 6-ccmp-cc-mode.patch Description: Binary data
Re: [Ping] [PATCH, 6/10] aarch64: add ccmp CC mode
On 10/27/2014 12:48 AM, Zhenqiang Chen wrote: -Original Message- From: Richard Henderson [mailto:r...@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. Um, no. The point of returning -1 is to avoid combining with comparisons for which we cannot produce the proper result. E.g. the existing CC_Zmode, where only the Z bit is valid. We want to reject combination with LTU, which checks the C bit. Are you honestly suggesting that using CC_DNEmode with GE can be made to make sense in any way? r~
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. 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; 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~
