This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Ping] [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
- From: Richard Henderson <rth at redhat dot com>
- To: Zhenqiang Chen <zhenqiang dot chen at arm dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 27 Oct 2014 09:02:54 -0700
- Subject: Re: [Ping] [PATCH, 10/10] aarch64: Handle ccmp in ifcvt to make it work with cmov
- Authentication-results: sourceware.org; auth=none
- References: <002101cfd6fa$259cb9f0$70d62dd0$ at arm dot com> <5439A3B7 dot 50803 at redhat dot com> <001701cff1ba$b88c0970$29a41c50$ at arm dot com>
On 10/27/2014 12:50 AM, Zhenqiang Chen wrote:
> Good point. It is not ccmp special. It is cbranchcc4 related. If I
> understand correct, without cbranchcc4, we need put the result to a tmp
> register and generate additional compares, which is not good for
> performance.
It won't be an additional compare. It is regenerating the compare at a new
location. The old comparison would be deleted via dead code elimination.
That said,
> +#if HAVE_cbranchcc4
> + allow_cc_mode = true;
> +#endif
does seem to be the right solution.
If a target has this, we ought to be able to reasonably expect that it's got
all the other patterns that this implies. If a target is missing them,
hopefully we can get that sorted fairly quickly after this patch is included.
> +#if HAVE_cbranchcc4
> + if (!(GET_MODE_CLASS (GET_MODE (cmp_a)) == MODE_CC
> + || GET_MODE_CLASS (GET_MODE (cmp_b)) == MODE_CC))
> +#endif
This test looks weird, considering what we're looking for.
I think a better test is
if (GET_MODE_CLASS (GET_MODE (cmp_a)) != MODE_CC
|| cmp_b != const0_rtx)
Accepting something like (compare (reg:CC a) (reg:CC b)) is definitely
non-canonical. Even (compare (const_int 0) (reg:CC flags)) is odd.
The ifcvt.c change should go in by itself.
The expr.c change should also be standalone.
The ccmp.c change should probably be merged with the initial commit of ccmp.c.
The aaarch64.md change should probably be merged with the patch that adds
cbranchcc.
r~