RE: [PATCH, AARCH64] Fix ICE in CCMP (PR64015)

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

> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: Tuesday, November 25, 2014 5:25 PM
> To: Zhenqiang Chen
> Cc: Marcus Shawcroft; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, AARCH64] Fix ICE in CCMP (PR64015)
> 
> On 11/25/2014 09:41 AM, Zhenqiang Chen wrote:
> > I want to confirm with you two things before I rework it.
> > (1) expand_insn needs an optab_handler as input. Do I need to define a
> ccmp_optab with different mode support in optabs.def?
> 
> No, look again: expand_insn needs an enum insn_code as input.  Since this is
> the backend, you can use any icode name you like, which means that you can
> use CODE_FOR_ccmp_and etc directly.
> 
> > (2) To make sure later operands not clobber CC, all operands are expanded
> before ccmp-first in current implementation. If taking tree/gimple as input,
> what's your preferred logic to guarantee CC not clobbered?
> 
> Hmm.  Perhaps the target hook will need to output two sequences, each of
> which will be concatenated while looping around the calls to gen_ccmp_next.
> The first sequence will be operand preparation and the second sequence will
> be ccmp generation.
> 
> Something like
> 
> bool
> aarch64_gen_ccmp_start(rtx *prep_seq, rtx *gen_seq,
>                        int cmp_code, int bit_code,
>                        tree op0, tree op1) {
>   bool success;
> 
>   start_sequence ();
>   // Widen and expand operands
>   *prep_seq = get_insns ();
>   end_sequence ();
> 
>   start_sequence ();
>   // Generate the first compare
>   *gen_seq = get_insns ();
>   end_sequence ();
> 
>   return success;
> }
> 
> bool
> aarch64_gen_ccmp_next(rtx *prep_seq, rtx *gen_seq,
>                       rtx prev, int cmp_code, int bit_code,
>                       tree op0, tree op1) {
>   bool success;
> 
>   push_to_sequence (*prep_seq);
>   // Widen and expand operands
>   *prep_seq = get_insns ();
>   end_sequence ();
> 
>   push_to_sequence (*gen_seq);
>   // Generate the next ccmp
>   *gen_seq = get_insns ();
>   end_sequence ();
> 
>   return success;
> }
> 
> If there are ever any failures, the middle-end can simply discard the
> sequences.  If everything succeeds, it simply calls emit_insn on both
> sequences.
> 
 
Thanks for the comments. The updated patch is attached.

Note: Function "aarch64_code_to_ccmode" is the same as it before reverting.

ChangeLog:
2014-12-12  Zhenqiang Chen  <zhenqiang.chen@arm.com>

	* ccmp.c (expand_ccmp_next): New function.
	(expand_ccmp_expr_1, expand_ccmp_expr): Handle operand insn sequence
	and compare insn sequence.
	* config/aarch64/aarch64.c (aarch64_code_to_ccmode,
	aarch64_gen_ccmp_first, aarch64_gen_ccmp_next): New functions.
	(TARGET_GEN_CCMP_FIRST, TARGET_GEN_CCMP_NEXT): New MICRO.
	* config/aarch64/aarch64.md (*ccmp_and): Changed to ccmp_and<mode>.
	(*ccmp_ior): Changed to ccmp_ior<mode>.
	(cmp<mode>): New pattern.
	* doc/tm.texi (TARGET_GEN_CCMP_FIRST, TARGET_GEN_CCMP_NEXT): Update
	parameters.
	* target.def (gen_ccmp_first, gen_ccmp_next): Update parameters.

testsuite/ChangeLog:
2014-12-12  Zhenqiang Chen  <zhenqiang.chen@arm.com>

	* gcc.dg/pr64015.c: New test.

Attachment: gen-ccmp.patch
Description: Binary data