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

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

[Andrew Pinski]

On Sun, Nov 23, 2014 at 9:11 PM, Zhenqiang Chen <zhenqiang.chen@arm.com> wrote:
> Hi,
>
> Expand pass always uses sign-extend to represent constant value. For the
> case in the patch, a 8-bit unsigned value "252" is represented as "-4",
> which pass the ccmn check. After mode conversion, "-4" becomes "252", which
> leads to mismatch.
>
> The patch adds another operand check after mode conversion.
>
> No make check regression with qemu.
>
> OK for trunk?
>
> Thanks!
> -Zhenqiang
>
> ChangeLog:
> 2014-11-24  Zhenqiang Chen  <zhenqiang.chen@arm.com>
>
>         PR target/64015
>         * config/aarch64/aarch64.c (aarch64_gen_ccmp_first): Recheck operand
>         after mode conversion.
>         (aarch64_gen_ccmp_next): Likewise.
>
> testsuite/ChangeLog:
> 2014-11-24  Zhenqiang Chen  <zhenqiang.chen@arm.com>
>
>         * gcc.target/aarch64/pr64015.c: New test.
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 1809513..203d095 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -10311,7 +10311,10 @@ aarch64_gen_ccmp_first (int code, rtx op0, rtx op1)
>    if (!aarch64_plus_operand (op1, GET_MODE (op1)))
>      op1 = force_reg (mode, op1);
>
> -  if (!aarch64_convert_mode (&op0, &op1, unsignedp))
> +  if (!aarch64_convert_mode (&op0, &op1, unsignedp)
> +        /* Some negative value might be transformed into a positive one.
> +           So need recheck here.  */
> +      || !aarch64_plus_operand (op1, GET_MODE (op1)))
>      return NULL_RTX;

Shouldn't we force it to a reg here instead?

>
>    mode = aarch64_code_to_ccmode ((enum rtx_code) code);
> @@ -10344,7 +10347,10 @@ aarch64_gen_ccmp_next (rtx prev, int cmp_code, rtx
> op0, rtx op1, int bit_code)
>        || !aarch64_ccmp_operand (op1, GET_MODE (op1)))
>      return NULL_RTX;
>
> -  if (!aarch64_convert_mode (&op0, &op1, unsignedp))
> +  if (!aarch64_convert_mode (&op0, &op1, unsignedp)
> +        /* Some negative value might be transformed into a positive one.
> +           So need recheck here.  */
> +      || !aarch64_ccmp_operand (op1, GET_MODE (op1)))
>      return NULL_RTX;


Also really the cost of forcing to a register is better really.
In the cases where we would not have forced to a register in a cmp
instruction, the constant would be one instruction and compared to the
cost of two cset and an and/or is better.
In the cases where we would have forced to a register for the cmp
instruction, two cost for doing the forcing is the same on both cases
but since we gaining from removing a cset and an and/or we are better.

>
>    mode = aarch64_code_to_ccmode ((enum rtx_code) cmp_code);
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr64015.c
> b/gcc/testsuite/gcc.target/aarch64/pr64015.c
> new file mode 100644
> index 0000000..eeed665
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr64015.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile } */
> +/* { dg-options " -O2 " } */
> +int
> +test (unsigned short a, unsigned char b)
> +{
> +  return a > 0xfff2 && b > 252;
> +}

Since this testcase is generic (except for the -O2), it really should
go into gcc.c-torture/compile instead of remove the two dg-*
directives so it can be tested on more than AARCH64 and on more
optimization levels.


Thanks,
Andrew Pinski

>
>
>

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

[Zhenqiang Chen]

Hi,

Expand pass always uses sign-extend to represent constant value. For the
case in the patch, a 8-bit unsigned value "252" is represented as "-4",
which pass the ccmn check. After mode conversion, "-4" becomes "252", which
leads to mismatch.

The patch adds another operand check after mode conversion.

No make check regression with qemu.

OK for trunk?

Thanks!
-Zhenqiang

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

	PR target/64015
	* config/aarch64/aarch64.c (aarch64_gen_ccmp_first): Recheck operand
	after mode conversion.
	(aarch64_gen_ccmp_next): Likewise.

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

	* gcc.target/aarch64/pr64015.c: New test.

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1809513..203d095 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10311,7 +10311,10 @@ aarch64_gen_ccmp_first (int code, rtx op0, rtx op1)
   if (!aarch64_plus_operand (op1, GET_MODE (op1)))
     op1 = force_reg (mode, op1);
 
-  if (!aarch64_convert_mode (&op0, &op1, unsignedp))
+  if (!aarch64_convert_mode (&op0, &op1, unsignedp)
+	 /* Some negative value might be transformed into a positive one.
+	    So need recheck here.  */
+      || !aarch64_plus_operand (op1, GET_MODE (op1)))
     return NULL_RTX;
 
   mode = aarch64_code_to_ccmode ((enum rtx_code) code);
@@ -10344,7 +10347,10 @@ aarch64_gen_ccmp_next (rtx prev, int cmp_code, rtx
op0, rtx op1, int bit_code)
       || !aarch64_ccmp_operand (op1, GET_MODE (op1)))
     return NULL_RTX;
 
-  if (!aarch64_convert_mode (&op0, &op1, unsignedp))
+  if (!aarch64_convert_mode (&op0, &op1, unsignedp)
+	 /* Some negative value might be transformed into a positive one.
+	    So need recheck here.  */
+      || !aarch64_ccmp_operand (op1, GET_MODE (op1)))
     return NULL_RTX;
 
   mode = aarch64_code_to_ccmode ((enum rtx_code) cmp_code);

diff --git a/gcc/testsuite/gcc.target/aarch64/pr64015.c
b/gcc/testsuite/gcc.target/aarch64/pr64015.c
new file mode 100644
index 0000000..eeed665
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr64015.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options " -O2 " } */
+int
+test (unsigned short a, unsigned char b)
+{
+  return a > 0xfff2 && b > 252;
+}

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

[Richard Henderson]

On 11/24/2014 06:11 AM, Zhenqiang Chen wrote:
> Expand pass always uses sign-extend to represent constant value. For the
> case in the patch, a 8-bit unsigned value "252" is represented as "-4",
> which pass the ccmn check. After mode conversion, "-4" becomes "252", which
> leads to mismatch.

This sort of thing is why I suggested from the beginning that expansion
happen directly from trees instead of sort-of re-expanding from rtl.

I think you're better off fixing this properly than hacking around it here.


r~

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

[Zhenqiang Chen]

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Richard Henderson
> Sent: Monday, November 24, 2014 4:57 PM
> To: Zhenqiang Chen; gcc-patches@gcc.gnu.org
> Cc: Marcus Shawcroft
> Subject: Re: [PATCH, AARCH64] Fix ICE in CCMP (PR64015)
> 
> On 11/24/2014 06:11 AM, Zhenqiang Chen wrote:
> > Expand pass always uses sign-extend to represent constant value. For
> > the case in the patch, a 8-bit unsigned value "252" is represented as
> > "-4", which pass the ccmn check. After mode conversion, "-4" becomes
> > "252", which leads to mismatch.
> 
> This sort of thing is why I suggested from the beginning that expansion
> happen directly from trees instead of sort-of re-expanding from rtl.
> 
> I think you're better off fixing this properly than hacking around it here.

Thanks for the comments.

Here was your previous comments: "We could avoid that by using struct expand_operand, create_input_operand et al, then expand_insn.  That does require that the target hooks be given trees rather than rtl as input."

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?
(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?

Thanks!
-Zhenqiang

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

[Richard Henderson]

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.


r~

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

[Zhenqiang Chen]

> -----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.

When there are more than one ccmps, it will be complexity to maintain the un-emitted sequences in a recursive algorithm. E.g.
     CC0 = CMP (a, b);
     CC1 = CCMP (NE (CC0, 0), CMP (e, f));
     ...
     CCn = CCMP (NE (CCn-1, 0), CMP (...));

I read the codes to expand cstoresi and cbranchsi. It just uses normal expand_operands. So I think we can keep expand_operands in middle-end.

For the start one, we do not need worry about it since its last step should compare to set CC. And it is easy to delete the insns when fail.
static rtx
aarch64_gen_ccmp_first (int code, rtx op0, rtx op1)
{
   ...  // init the vars

  op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode, unsignedp);
  op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode, unsignedp);
  if (!op0 || !op1)
    return NULL_RTX;

  cmp = gen_rtx_fmt_ee ((enum rtx_code) code, cmp_mode, op0, op1);
  target = gen_rtx_REG (CCmode, CC_REGNUM);

  create_output_operand (&ops[0], target, CCmode);
  create_fixed_operand (&ops[1], cmp);
  create_fixed_operand (&ops[2], op0);
  create_fixed_operand (&ops[3], op1);

  if (!maybe_expand_insn (icode, 4, ops))
    return NULL_RTX;

  return gen_rtx_REG (cc_mode, CC_REGNUM);
}

aarch64_gen_ccmp_next (rtx prev, int cmp_code, rtx op0, rtx op1, int bit_code)
{
   ...  // init the vars

  op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode, unsignedp);
  op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode, unsignedp);
  if (!op0 || !op1)
    return NULL_RTX;

  /* Check to make sure CC is not clobbered since prepare_operand might
     generates copy or mode convertion insns, although no test shows
     such insns clobber CC.  */
  ...

  cmp1 = gen_rtx_fmt_ee ((enum rtx_code) cmp_code, cmp_mode, op0, op1);
  cmp0 = gen_rtx_fmt_ee (NE, cmp_mode, prev, const0_rtx);

  target = gen_rtx_REG (cc_mode, CC_REGNUM);

  create_fixed_operand (&ops[0], prev);
  create_fixed_operand (&ops[1], target);
  create_fixed_operand (&ops[2], op0);
  create_fixed_operand (&ops[3], op1);
  create_fixed_operand (&ops[4], cmp0);
  create_fixed_operand (&ops[5], cmp1);

  if (!maybe_expand_insn (icode, 6, ops))
    return NULL_RTX;

  return target;
}

Does such change align with your comments?

Thanks!
-Zhenqiang

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

[Richard Henderson]

On 11/26/2014 10:23 AM, Zhenqiang Chen wrote:
>   /* Check to make sure CC is not clobbered since prepare_operand might
>      generates copy or mode convertion insns, although no test shows
>      such insns clobber CC.  */

And what do you do if a clobber does happen?
With TER enabled, there's every possibility that it might be.

> Does such change align with your comments?

Not really, no.


r~

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

[Zhenqiang Chen]

[-- Attachment #1: Type: text/plain, Size: 3241 bytes --]



> -----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 #2: gen-ccmp.patch --]
[-- Type: application/octet-stream, Size: 18914 bytes --]

diff --git a/gcc/ccmp.c b/gcc/ccmp.c
index 9c239e2..f26bc36 100644
--- a/gcc/ccmp.c
+++ b/gcc/ccmp.c
@@ -68,7 +68,16 @@ along with GCC; see the file COPYING3.  If not see
 
      * If the final result is not used in a COND_EXPR (checked by function
        used_in_cond_stmt_p), it calls cstorecc4 pattern to store the CC to a
-       general register.  */
+       general register.
+
+   Since the operands of the later compares might clobber CC reg, we do not
+   emit the insns during expand.  We keep the insn sequences in two seq
+
+     * prep_seq, which includes all the insns to prepare the operands.
+     * gen_seq, which includes all the compare and conditional compares.
+
+   If all checks OK in expand_ccmp_expr, it emits insns in prep_seq, then
+   insns in gen_seq.  */
 
 /* Check whether G is a potential conditional compare candidate.  */
 static bool
@@ -148,6 +157,27 @@ used_in_cond_stmt_p (tree exp)
   return expand_cond;
 }
 
+/* PREV is the CC flag from precvious compares.  The function expands the
+   next compare based on G which ops previous compare with CODE.
+   PREP_SEQ returns all insns to prepare opearands for compare.
+   GEN_SEQ returnss all compare insns.  */
+static rtx
+expand_ccmp_next (gimple g, enum tree_code code, rtx prev,
+		  rtx *prep_seq, rtx *gen_seq)
+{
+  enum rtx_code rcode;
+  int unsignedp = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (g)));
+
+  gcc_assert (code == BIT_AND_EXPR || code == BIT_IOR_EXPR);
+
+  rcode = get_rtx_code (gimple_assign_rhs_code (g), unsignedp);
+
+  return targetm.gen_ccmp_next (prep_seq, gen_seq, prev, rcode,
+				gimple_assign_rhs1 (g),
+				gimple_assign_rhs2 (g),
+				get_rtx_code (code, 0));
+}
+
 /* Expand conditional compare gimple G.  A typical CCMP sequence is like:
 
      CC0 = CMP (a, b);
@@ -156,9 +186,11 @@ used_in_cond_stmt_p (tree exp)
      CCn = CCMP (NE (CCn-1, 0), CMP (...));
 
    hook gen_ccmp_first is used to expand the first compare.
-   hook gen_ccmp_next is used to expand the following CCMP.  */
+   hook gen_ccmp_next is used to expand the following CCMP.
+   PREP_SEQ returns all insns to prepare opearand.
+   GEN_SEQ returns all compare insns.  */
 static rtx
-expand_ccmp_expr_1 (gimple g)
+expand_ccmp_expr_1 (gimple g, rtx *prep_seq, rtx *gen_seq)
 {
   tree exp = gimple_assign_rhs_to_tree (g);
   enum tree_code code = TREE_CODE (exp);
@@ -175,52 +207,27 @@ expand_ccmp_expr_1 (gimple g)
     {
       if (TREE_CODE_CLASS (code1) == tcc_comparison)
 	{
-	  int unsignedp0, unsignedp1;
-	  enum rtx_code rcode0, rcode1;
-	  rtx op0, op1, op2, op3, tmp;
+	  int unsignedp0;
+	  enum rtx_code rcode0;
 
 	  unsignedp0 = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs0)));
 	  rcode0 = get_rtx_code (code0, unsignedp0);
-	  unsignedp1 = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs1)));
-	  rcode1 = get_rtx_code (code1, unsignedp1);
-
-	  expand_operands (gimple_assign_rhs1 (gs0),
-			   gimple_assign_rhs2 (gs0),
-			   NULL_RTX, &op0, &op1, EXPAND_NORMAL);
-
-	  /* Since the operands of GS1 might clobber CC reg, we expand the
-	     operands of GS1 before GEN_CCMP_FIRST.  */
-	  expand_operands (gimple_assign_rhs1 (gs1),
-			   gimple_assign_rhs2 (gs1),
-			   NULL_RTX, &op2, &op3, EXPAND_NORMAL);
-	  tmp = targetm.gen_ccmp_first (rcode0, op0, op1);
+
+	  tmp = targetm.gen_ccmp_first (prep_seq, gen_seq, rcode0,
+					gimple_assign_rhs1 (gs0),
+					gimple_assign_rhs2 (gs0));
 	  if (!tmp)
 	    return NULL_RTX;
 
-	  return targetm.gen_ccmp_next (tmp, rcode1, op2, op3,
-					get_rtx_code (code, 0));
+	  return expand_ccmp_next (gs1, code, tmp, prep_seq, gen_seq);
 	}
       else
 	{
-  	  rtx op0, op1;
-	  enum rtx_code rcode;
-	  int unsignedp = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs0)));
-
-	  rcode = get_rtx_code (gimple_assign_rhs_code (gs0), unsignedp);
-
-	  /* Hoist the preparation operations above the entire
-	     conditional compare sequence.  */
-	  expand_operands (gimple_assign_rhs1 (gs0),
-			   gimple_assign_rhs2 (gs0),
-			   NULL_RTX, &op0, &op1, EXPAND_NORMAL);
-
-	  gcc_assert (code1 == BIT_AND_EXPR || code1 == BIT_IOR_EXPR);
+	  tmp = expand_ccmp_expr_1 (gs1, prep_seq, gen_seq);
+	  if (!tmp)
+	    return NULL_RTX;
 
-	  /* Note: We swap the order to make the recursive function work.  */
-	  tmp = expand_ccmp_expr_1 (gs1);
-	  if (tmp)
-	    return targetm.gen_ccmp_next (tmp, rcode, op0, op1,
-					  get_rtx_code (code, 0));
+	  return expand_ccmp_next (gs0, code, tmp, prep_seq, gen_seq);
 	}
     }
   else
@@ -230,21 +237,11 @@ expand_ccmp_expr_1 (gimple g)
 
       if (TREE_CODE_CLASS (gimple_assign_rhs_code (gs1)) == tcc_comparison)
 	{
-  	  rtx op0, op1;
-	  enum rtx_code rcode;
-	  int unsignedp = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs1)));
-
-	  rcode = get_rtx_code (gimple_assign_rhs_code (gs1), unsignedp);
-
-	  /* Hoist the preparation operations above the entire
-	     conditional compare sequence.  */
-	  expand_operands (gimple_assign_rhs1 (gs1),
-			   gimple_assign_rhs2 (gs1),
-			   NULL_RTX, &op0, &op1, EXPAND_NORMAL);
-	  tmp = expand_ccmp_expr_1 (gs0);
-	  if (tmp)
-	    return targetm.gen_ccmp_next (tmp, rcode, op0, op1,
-					  get_rtx_code (code, 0));
+	  tmp = expand_ccmp_expr_1 (gs0, prep_seq, gen_seq);
+	  if (!tmp)
+	    return NULL_RTX;
+
+	  return expand_ccmp_next (gs1, code, tmp, prep_seq, gen_seq);
 	}
       else
 	{
@@ -264,23 +261,30 @@ expand_ccmp_expr (gimple g)
 {
   rtx_insn *last;
   rtx tmp;
+  rtx prep_seq, gen_seq;
+
+  prep_seq = gen_seq = NULL_RTX;
 
   if (!ccmp_candidate_p (g))
     return NULL_RTX;
 
   last = get_last_insn ();
-  tmp = expand_ccmp_expr_1 (g);
+  tmp = expand_ccmp_expr_1 (g, &prep_seq, &gen_seq);
 
   if (tmp)
     {
       enum insn_code icode;
       enum machine_mode cc_mode = CCmode;
-
       tree lhs = gimple_assign_lhs (g);
+
       /* TMP should be CC.  If it is used in a GIMPLE_COND, just return it.
 	 Note: Target needs to define "cbranchcc4".  */
       if (used_in_cond_stmt_p (lhs))
-	return tmp;
+	{
+	  emit_insn (prep_seq);
+	  emit_insn (gen_seq);
+	  return tmp;
+	}
 
 #ifdef SELECT_CC_MODE
       cc_mode = SELECT_CC_MODE (NE, tmp, const0_rtx);
@@ -290,13 +294,22 @@ expand_ccmp_expr (gimple g)
       icode = optab_handler (cstore_optab, cc_mode);
       if (icode != CODE_FOR_nothing)
 	{
-	  tree lhs = gimple_assign_lhs (g);
 	  enum machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
 	  rtx target = gen_reg_rtx (mode);
+
+	  start_sequence ();
 	  tmp = emit_cstore (target, icode, NE, cc_mode, cc_mode,
 			     0, tmp, const0_rtx, 1, mode);
 	  if (tmp)
-	    return tmp;
+	    {
+	      rtx seq = get_insns ();
+	      end_sequence ();
+	      emit_insn (prep_seq);
+	      emit_insn (gen_seq);
+	      emit_insn (seq);
+	      return tmp;
+	    }
+	  end_sequence ();
 	}
     }
   /* Clean up.  */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 9f7cccc..31aa970 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10244,6 +10244,199 @@ aarch64_use_by_pieces_infrastructure_p (unsigned HOST_WIDE_INT size,
   return default_use_by_pieces_infrastructure_p (size, align, op, speed_p);
 }
 
+static enum machine_mode
+aarch64_code_to_ccmode (enum rtx_code code)
+{
+  switch (code)
+    {
+    case NE:
+      return CC_DNEmode;
+
+    case EQ:
+      return CC_DEQmode;
+
+    case LE:
+      return CC_DLEmode;
+
+    case LT:
+      return CC_DLTmode;
+
+    case GE:
+      return CC_DGEmode;
+
+    case GT:
+      return CC_DGTmode;
+
+    case LEU:
+      return CC_DLEUmode;
+
+    case LTU:
+      return CC_DLTUmode;
+
+    case GEU:
+      return CC_DGEUmode;
+
+    case GTU:
+      return CC_DGTUmode;
+
+    default:
+      return CCmode;
+    }
+}
+
+static rtx
+aarch64_gen_ccmp_first (rtx *prep_seq, rtx *gen_seq,
+			int code, tree treeop0, tree treeop1)
+{
+  enum machine_mode op_mode, cmp_mode, cc_mode;
+  rtx op0, op1, cmp, target;
+  int unsignedp = code == LTU || code == LEU || code == GTU || code == GEU;
+  enum insn_code icode;
+  struct expand_operand ops[4];
+
+  cc_mode = aarch64_code_to_ccmode ((enum rtx_code) code);
+  if (cc_mode == CCmode)
+    return NULL_RTX;
+
+  start_sequence ();
+  expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1, EXPAND_NORMAL);
+
+  op_mode = GET_MODE (op0);
+  if (op_mode == VOIDmode)
+    op_mode = GET_MODE (op1);
+
+  switch (op_mode)
+    {
+    case QImode:
+    case HImode:
+    case SImode:
+      cmp_mode = SImode;
+      icode = CODE_FOR_cmpsi;
+      break;
+
+    case DImode:
+      cmp_mode = DImode;
+      icode = CODE_FOR_cmpdi;
+      break;
+
+    default:
+      end_sequence ();
+      return NULL_RTX;
+    }
+
+  op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode, unsignedp);
+  op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode, unsignedp);
+  if (!op0 || !op1)
+    {
+      end_sequence ();
+      return NULL_RTX;
+    }
+  *prep_seq = get_insns ();
+  end_sequence ();
+
+  cmp = gen_rtx_fmt_ee ((enum rtx_code) code, cmp_mode, op0, op1);
+  target = gen_rtx_REG (CCmode, CC_REGNUM);
+
+  create_output_operand (&ops[0], target, CCmode);
+  create_fixed_operand (&ops[1], cmp);
+  create_fixed_operand (&ops[2], op0);
+  create_fixed_operand (&ops[3], op1);
+
+  start_sequence ();
+  if (!maybe_expand_insn (icode, 4, ops))
+    {
+      end_sequence ();
+      return NULL_RTX;
+    }
+  *gen_seq = get_insns ();
+  end_sequence ();
+
+  return gen_rtx_REG (cc_mode, CC_REGNUM);
+}
+
+static rtx
+aarch64_gen_ccmp_next (rtx *prep_seq, rtx *gen_seq, rtx prev, int cmp_code,
+		       tree treeop0, tree treeop1, int bit_code)
+{
+  rtx op0, op1, cmp0, cmp1, target;
+  enum machine_mode op_mode, cmp_mode, cc_mode;
+  int unsignedp = cmp_code == LTU || cmp_code == LEU
+		  || cmp_code == GTU || cmp_code == GEU;
+  enum insn_code icode = CODE_FOR_ccmp_andsi;
+  struct expand_operand ops[6];
+
+  cc_mode = aarch64_code_to_ccmode ((enum rtx_code) cmp_code);
+  if (cc_mode == CCmode)
+    return NULL_RTX;
+
+  push_to_sequence ((rtx_insn*) *prep_seq);
+  expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1, EXPAND_NORMAL);
+
+  op_mode = GET_MODE (op0);
+  if (op_mode == VOIDmode)
+    op_mode = GET_MODE (op1);
+
+  switch (op_mode)
+    {
+    case QImode:
+    case HImode:
+    case SImode:
+      cmp_mode = SImode;
+      icode = (enum rtx_code) bit_code == AND ? CODE_FOR_ccmp_andsi
+						: CODE_FOR_ccmp_iorsi;
+      break;
+
+    case DImode:
+      cmp_mode = DImode;
+      icode = (enum rtx_code) bit_code == AND ? CODE_FOR_ccmp_anddi
+						: CODE_FOR_ccmp_iordi;
+      break;
+
+    default:
+      end_sequence ();
+      return NULL_RTX;
+    }
+
+  op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode, unsignedp);
+  op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode, unsignedp);
+  if (!op0 || !op1)
+    {
+      end_sequence ();
+      return NULL_RTX;
+    }
+  *prep_seq = get_insns ();
+  end_sequence ();
+
+  target = gen_rtx_REG (cc_mode, CC_REGNUM);
+  cmp1 = gen_rtx_fmt_ee ((enum rtx_code) cmp_code, cmp_mode, op0, op1);
+  cmp0 = gen_rtx_fmt_ee (NE, cmp_mode, prev, const0_rtx);
+
+  create_fixed_operand (&ops[0], prev);
+  create_fixed_operand (&ops[1], target);
+  create_fixed_operand (&ops[2], op0);
+  create_fixed_operand (&ops[3], op1);
+  create_fixed_operand (&ops[4], cmp0);
+  create_fixed_operand (&ops[5], cmp1);
+
+  push_to_sequence ((rtx_insn*) *gen_seq);
+  if (!maybe_expand_insn (icode, 6, ops))
+    {
+      end_sequence ();
+      return NULL_RTX;
+    }
+
+  *gen_seq = get_insns ();
+  end_sequence ();
+
+  return target;
+}
+
+#undef TARGET_GEN_CCMP_FIRST
+#define TARGET_GEN_CCMP_FIRST aarch64_gen_ccmp_first
+
+#undef TARGET_GEN_CCMP_NEXT
+#define TARGET_GEN_CCMP_NEXT aarch64_gen_ccmp_next
+
 /* Implement TARGET_SCHED_MACRO_FUSION_P.  Return true if target supports
    instruction fusion of some sort.  */
 
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 597ff8c..d13231e 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -256,7 +256,7 @@
   ""
   "")
 
-(define_insn "*ccmp_and"
+(define_insn "ccmp_and<mode>"
   [(set (match_operand 1 "ccmp_cc_register" "")
 	(compare
 	 (and:SI
@@ -275,7 +275,7 @@
   [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
 )
 
-(define_insn "*ccmp_ior"
+(define_insn "ccmp_ior<mode>"
   [(set (match_operand 1 "ccmp_cc_register" "")
 	(compare
 	 (ior:SI
@@ -294,6 +294,20 @@
   [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
 )
 
+(define_expand "cmp<mode>"
+  [(set (match_operand 0 "cc_register" "")
+        (match_operator:CC 1 "aarch64_comparison_operator"
+         [(match_operand:GPI 2 "register_operand" "")
+          (match_operand:GPI 3 "aarch64_plus_operand" "")]))]
+  ""
+  "
+  operands[1] = gen_rtx_fmt_ee (COMPARE, SELECT_CC_MODE (GET_CODE (operands[1]),
+							 operands[2],
+							 operands[3]),
+				 operands[2], operands[3]);
+  "
+)
+
 (define_insn "*condjump"
   [(set (pc) (if_then_else (match_operator 0 "aarch64_comparison_operator"
 			    [(match_operand 1 "cc_register" "") (const_int 0)])
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index c54fc71..4ef4409 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11234,18 +11234,25 @@ This target hook is required only when the target has several different
 modes and they have different conditional execution capability, such as ARM.
 @end deftypefn
 
-@deftypefn {Target Hook} rtx TARGET_GEN_CCMP_FIRST (int @var{code}, rtx @var{op0}, rtx @var{op1})
-This function emits a comparison insn for the first of a sequence of
- conditional comparisions.  It returns a comparison expression appropriate
- for passing to @code{gen_ccmp_next} or @code{cbranch_optab}.  @var{code} is
+@deftypefn {Target Hook} rtx TARGET_GEN_CCMP_FIRST (rtx *@var{prep_seq}, rtx *@var{gen_seq}, int @var{code}, tree @var{op0}, tree @var{op1})
+This function prepares to emit a comparison insn for the first compare in a
+ sequence of conditional comparisions.  It returns a appropriate @code{CC}
+ for passing to @code{gen_ccmp_next} or @code{cbranch_optab}.  The insns to
+ prepare the compare are saved in @var{prep_seq} and the compare insns are
+ saved in @var{gen_seq}.  They will be emitted when all the compares in the
+ the conditional comparision are generated without error.  @var{code} is
  the @code{rtx_code} of the compare for @var{op0} and @var{op1}.
 @end deftypefn
 
-@deftypefn {Target Hook} rtx TARGET_GEN_CCMP_NEXT (rtx @var{prev}, int @var{cmp_code}, rtx @var{op0}, rtx @var{op1}, int @var{bit_code})
-This function emits a conditional comparison within a sequence of
- conditional comparisons.  The @var{prev} expression is the result of a
- prior call to @code{gen_ccmp_first} or @code{gen_ccmp_next}.  It may return
- @code{NULL} if the combination of @var{prev} and this comparison is
+@deftypefn {Target Hook} rtx TARGET_GEN_CCMP_NEXT (rtx *@var{prep_seq}, rtx *@var{gen_seq}, rtx @var{prev}, int @var{cmp_code}, tree @var{op0}, tree @var{op1}, int @var{bit_code})
+This function prepare to emit a conditional comparison within a sequence of
+ conditional comparisons.  It returns a appropriate @code{CC} for passing to
+ @code{gen_ccmp_next} or @code{cbranch_optab}.  The insns to prepare the
+ compare are saved in @var{prep_seq} and the compare insns are saved in
+ @var{gen_seq}.  They will be emitted when all the compares in the conditional
+ comparision are generated without error.  The @var{prev} expression is the
+ result of a prior call to @code{gen_ccmp_first} or @code{gen_ccmp_next}.  It
+ may return @code{NULL} if the combination of @var{prev} and this comparison is
  not supported, otherwise the result must be appropriate for passing to
  @code{gen_ccmp_next} or @code{cbranch_optab}.  @var{code} is the
  @code{rtx_code} of the compare for @var{op0} and @var{op1}.  @var{bit_code}
diff --git a/gcc/target.def b/gcc/target.def
index 7c0296d..3770f37 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2536,24 +2536,31 @@ modes and they have different conditional execution capability, such as ARM.",
 
 DEFHOOK
 (gen_ccmp_first,
- "This function emits a comparison insn for the first of a sequence of\n\
- conditional comparisions.  It returns a comparison expression appropriate\n\
- for passing to @code{gen_ccmp_next} or @code{cbranch_optab}.  @var{code} is\n\
+ "This function prepares to emit a comparison insn for the first compare in a\n\
+ sequence of conditional comparisions.  It returns a appropriate @code{CC}\n\
+ for passing to @code{gen_ccmp_next} or @code{cbranch_optab}.  The insns to\n\
+ prepare the compare are saved in @var{prep_seq} and the compare insns are\n\
+ saved in @var{gen_seq}.  They will be emitted when all the compares in the\n\
+ the conditional comparision are generated without error.  @var{code} is\n\
  the @code{rtx_code} of the compare for @var{op0} and @var{op1}.",
- rtx, (int code, rtx op0, rtx op1),
+ rtx, (rtx *prep_seq, rtx *gen_seq, int code, tree op0, tree op1),
  NULL)
 
 DEFHOOK
 (gen_ccmp_next,
- "This function emits a conditional comparison within a sequence of\n\
- conditional comparisons.  The @var{prev} expression is the result of a\n\
- prior call to @code{gen_ccmp_first} or @code{gen_ccmp_next}.  It may return\n\
- @code{NULL} if the combination of @var{prev} and this comparison is\n\
+ "This function prepare to emit a conditional comparison within a sequence of\n\
+ conditional comparisons.  It returns a appropriate @code{CC} for passing to\n\
+ @code{gen_ccmp_next} or @code{cbranch_optab}.  The insns to prepare the\n\
+ compare are saved in @var{prep_seq} and the compare insns are saved in\n\
+ @var{gen_seq}.  They will be emitted when all the compares in the conditional\n\
+ comparision are generated without error.  The @var{prev} expression is the\n\
+ result of a prior call to @code{gen_ccmp_first} or @code{gen_ccmp_next}.  It\n\
+ may return @code{NULL} if the combination of @var{prev} and this comparison is\n\
  not supported, otherwise the result must be appropriate for passing to\n\
  @code{gen_ccmp_next} or @code{cbranch_optab}.  @var{code} is the\n\
  @code{rtx_code} of the compare for @var{op0} and @var{op1}.  @var{bit_code}\n\
  is @code{AND} or @code{IOR}, which is the op on the two compares.",
- rtx, (rtx prev, int cmp_code, rtx op0, rtx op1, int bit_code),
+ rtx, (rtx *prep_seq, rtx *gen_seq, rtx prev, int cmp_code, tree op0, tree op1, int bit_code),
  NULL)
 
 /* Return a new value for loop unroll size.  */
diff --git a/gcc/testsuite/gcc.dg/pr64015.c b/gcc/testsuite/gcc.dg/pr64015.c
new file mode 100644
index 0000000..daf8393
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr64015.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 " } */
+
+int
+test (unsigned short a, unsigned char b)
+{
+  return a > 0xfff2 && b > 252;
+}
+
+/* { dg-final { scan-assembler "ccmp" { target aarch64*-*-* } } } */

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

[Richard Henderson]

> -	  tree lhs = gimple_assign_lhs (g);
>  	  enum machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
>  	  rtx target = gen_reg_rtx (mode);
> +
> +	  start_sequence ();
>  	  tmp = emit_cstore (target, icode, NE, cc_mode, cc_mode,
>  			     0, tmp, const0_rtx, 1, mode);
>  	  if (tmp)
> -	    return tmp;
> +	    {
> +	      rtx seq = get_insns ();
> +	      end_sequence ();
> +	      emit_insn (prep_seq);
> +	      emit_insn (gen_seq);
> +	      emit_insn (seq);
> +	      return tmp;
> +	    }
> +	  end_sequence ();

Given that you're already doing delete_insns_since (last)
at the end of this function, I don't think you need a new
sequence around the emit_cstore.  Just

	emit_insn (prep_seq);
	emit_insn (gen_seq);
	tmp = emit_cstore (...);
	if (tmp)
	  return tmp;

> +  int unsignedp = code == LTU || code == LEU || code == GTU || code == GEU;

You don't need to examine the code, you can look at the argument:

  TYPE_UNSIGNED (TREE_TYPE (treeop0))



> +  op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode, unsignedp);
> +  op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode, unsignedp);
> +  if (!op0 || !op1)
> +    {
> +      end_sequence ();
> +      return NULL_RTX;
> +    }
> +  *prep_seq = get_insns ();
> +  end_sequence ();
> +
> +  cmp = gen_rtx_fmt_ee ((enum rtx_code) code, cmp_mode, op0, op1);
> +  target = gen_rtx_REG (CCmode, CC_REGNUM);
> +
> +  create_output_operand (&ops[0], target, CCmode);
> +  create_fixed_operand (&ops[1], cmp);
> +  create_fixed_operand (&ops[2], op0);
> +  create_fixed_operand (&ops[3], op1);

Hmm.  With so many fixed operands, I think you may be better off not
creating the cmp<mode> expander in the first place.  Just inline the
SELECT_CC_MODE and everything right here.


r~

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

[Zhenqiang Chen]

[-- Attachment #1: Type: text/plain, Size: 2486 bytes --]



> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: Saturday, December 13, 2014 3:26 AM
> To: Zhenqiang Chen
> Cc: Marcus Shawcroft; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH, AARCH64] Fix ICE in CCMP (PR64015)
> 
> > -	  tree lhs = gimple_assign_lhs (g);
> >  	  enum machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
> >  	  rtx target = gen_reg_rtx (mode);
> > +
> > +	  start_sequence ();
> >  	  tmp = emit_cstore (target, icode, NE, cc_mode, cc_mode,
> >  			     0, tmp, const0_rtx, 1, mode);
> >  	  if (tmp)
> > -	    return tmp;
> > +	    {
> > +	      rtx seq = get_insns ();
> > +	      end_sequence ();
> > +	      emit_insn (prep_seq);
> > +	      emit_insn (gen_seq);
> > +	      emit_insn (seq);
> > +	      return tmp;
> > +	    }
> > +	  end_sequence ();
> 
> Given that you're already doing delete_insns_since (last) at the end of this
> function, I don't think you need a new sequence around the emit_cstore.
> Just
> 
> 	emit_insn (prep_seq);
> 	emit_insn (gen_seq);
> 	tmp = emit_cstore (...);
> 	if (tmp)
> 	  return tmp;

Updated.
 
> > +  int unsignedp = code == LTU || code == LEU || code == GTU || code
> > + == GEU;
> 
> You don't need to examine the code, you can look at the argument:
> 
>   TYPE_UNSIGNED (TREE_TYPE (treeop0))
 
Updated. 
 
> > +  op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode,
> > + unsignedp);
> > +  op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode,
> > + unsignedp);  if (!op0 || !op1)
> > +    {
> > +      end_sequence ();
> > +      return NULL_RTX;
> > +    }
> > +  *prep_seq = get_insns ();
> > +  end_sequence ();
> > +
> > +  cmp = gen_rtx_fmt_ee ((enum rtx_code) code, cmp_mode, op0, op1);
> > + target = gen_rtx_REG (CCmode, CC_REGNUM);
> > +
> > +  create_output_operand (&ops[0], target, CCmode);
> > + create_fixed_operand (&ops[1], cmp);  create_fixed_operand (&ops[2],
> > + op0);  create_fixed_operand (&ops[3], op1);
> 
> Hmm.  With so many fixed operands, I think you may be better off not
> creating the cmp<mode> expander in the first place.  Just inline the
> SELECT_CC_MODE and everything right here.

In the patch, I use prepare_operand (icode, op0, 2, ...) to do the operand MODE conversion (from HI/QI to SI), which needs a cmp<mode> expander. Without it, I have to add additional codes to do the conversion (as it in previous patch, which leads to PR64015).

Thanks!
-Zhenqiang

[-- Attachment #2: gen-ccmp-v2.patch --]
[-- Type: application/octet-stream, Size: 18633 bytes --]

diff --git a/gcc/ccmp.c b/gcc/ccmp.c
index 9c239e2..8878719 100644
--- a/gcc/ccmp.c
+++ b/gcc/ccmp.c
@@ -68,7 +68,16 @@ along with GCC; see the file COPYING3.  If not see
 
      * If the final result is not used in a COND_EXPR (checked by function
        used_in_cond_stmt_p), it calls cstorecc4 pattern to store the CC to a
-       general register.  */
+       general register.
+
+   Since the operands of the later compares might clobber CC reg, we do not
+   emit the insns during expand.  We keep the insn sequences in two seq
+
+     * prep_seq, which includes all the insns to prepare the operands.
+     * gen_seq, which includes all the compare and conditional compares.
+
+   If all checks OK in expand_ccmp_expr, it emits insns in prep_seq, then
+   insns in gen_seq.  */
 
 /* Check whether G is a potential conditional compare candidate.  */
 static bool
@@ -148,6 +157,27 @@ used_in_cond_stmt_p (tree exp)
   return expand_cond;
 }
 
+/* PREV is the CC flag from precvious compares.  The function expands the
+   next compare based on G which ops previous compare with CODE.
+   PREP_SEQ returns all insns to prepare opearands for compare.
+   GEN_SEQ returnss all compare insns.  */
+static rtx
+expand_ccmp_next (gimple g, enum tree_code code, rtx prev,
+		  rtx *prep_seq, rtx *gen_seq)
+{
+  enum rtx_code rcode;
+  int unsignedp = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (g)));
+
+  gcc_assert (code == BIT_AND_EXPR || code == BIT_IOR_EXPR);
+
+  rcode = get_rtx_code (gimple_assign_rhs_code (g), unsignedp);
+
+  return targetm.gen_ccmp_next (prep_seq, gen_seq, prev, rcode,
+				gimple_assign_rhs1 (g),
+				gimple_assign_rhs2 (g),
+				get_rtx_code (code, 0));
+}
+
 /* Expand conditional compare gimple G.  A typical CCMP sequence is like:
 
      CC0 = CMP (a, b);
@@ -156,9 +186,11 @@ used_in_cond_stmt_p (tree exp)
      CCn = CCMP (NE (CCn-1, 0), CMP (...));
 
    hook gen_ccmp_first is used to expand the first compare.
-   hook gen_ccmp_next is used to expand the following CCMP.  */
+   hook gen_ccmp_next is used to expand the following CCMP.
+   PREP_SEQ returns all insns to prepare opearand.
+   GEN_SEQ returns all compare insns.  */
 static rtx
-expand_ccmp_expr_1 (gimple g)
+expand_ccmp_expr_1 (gimple g, rtx *prep_seq, rtx *gen_seq)
 {
   tree exp = gimple_assign_rhs_to_tree (g);
   enum tree_code code = TREE_CODE (exp);
@@ -175,52 +207,27 @@ expand_ccmp_expr_1 (gimple g)
     {
       if (TREE_CODE_CLASS (code1) == tcc_comparison)
 	{
-	  int unsignedp0, unsignedp1;
-	  enum rtx_code rcode0, rcode1;
-	  rtx op0, op1, op2, op3, tmp;
+	  int unsignedp0;
+	  enum rtx_code rcode0;
 
 	  unsignedp0 = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs0)));
 	  rcode0 = get_rtx_code (code0, unsignedp0);
-	  unsignedp1 = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs1)));
-	  rcode1 = get_rtx_code (code1, unsignedp1);
-
-	  expand_operands (gimple_assign_rhs1 (gs0),
-			   gimple_assign_rhs2 (gs0),
-			   NULL_RTX, &op0, &op1, EXPAND_NORMAL);
-
-	  /* Since the operands of GS1 might clobber CC reg, we expand the
-	     operands of GS1 before GEN_CCMP_FIRST.  */
-	  expand_operands (gimple_assign_rhs1 (gs1),
-			   gimple_assign_rhs2 (gs1),
-			   NULL_RTX, &op2, &op3, EXPAND_NORMAL);
-	  tmp = targetm.gen_ccmp_first (rcode0, op0, op1);
+
+	  tmp = targetm.gen_ccmp_first (prep_seq, gen_seq, rcode0,
+					gimple_assign_rhs1 (gs0),
+					gimple_assign_rhs2 (gs0));
 	  if (!tmp)
 	    return NULL_RTX;
 
-	  return targetm.gen_ccmp_next (tmp, rcode1, op2, op3,
-					get_rtx_code (code, 0));
+	  return expand_ccmp_next (gs1, code, tmp, prep_seq, gen_seq);
 	}
       else
 	{
-  	  rtx op0, op1;
-	  enum rtx_code rcode;
-	  int unsignedp = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs0)));
-
-	  rcode = get_rtx_code (gimple_assign_rhs_code (gs0), unsignedp);
-
-	  /* Hoist the preparation operations above the entire
-	     conditional compare sequence.  */
-	  expand_operands (gimple_assign_rhs1 (gs0),
-			   gimple_assign_rhs2 (gs0),
-			   NULL_RTX, &op0, &op1, EXPAND_NORMAL);
-
-	  gcc_assert (code1 == BIT_AND_EXPR || code1 == BIT_IOR_EXPR);
+	  tmp = expand_ccmp_expr_1 (gs1, prep_seq, gen_seq);
+	  if (!tmp)
+	    return NULL_RTX;
 
-	  /* Note: We swap the order to make the recursive function work.  */
-	  tmp = expand_ccmp_expr_1 (gs1);
-	  if (tmp)
-	    return targetm.gen_ccmp_next (tmp, rcode, op0, op1,
-					  get_rtx_code (code, 0));
+	  return expand_ccmp_next (gs0, code, tmp, prep_seq, gen_seq);
 	}
     }
   else
@@ -230,21 +237,11 @@ expand_ccmp_expr_1 (gimple g)
 
       if (TREE_CODE_CLASS (gimple_assign_rhs_code (gs1)) == tcc_comparison)
 	{
-  	  rtx op0, op1;
-	  enum rtx_code rcode;
-	  int unsignedp = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs1)));
-
-	  rcode = get_rtx_code (gimple_assign_rhs_code (gs1), unsignedp);
-
-	  /* Hoist the preparation operations above the entire
-	     conditional compare sequence.  */
-	  expand_operands (gimple_assign_rhs1 (gs1),
-			   gimple_assign_rhs2 (gs1),
-			   NULL_RTX, &op0, &op1, EXPAND_NORMAL);
-	  tmp = expand_ccmp_expr_1 (gs0);
-	  if (tmp)
-	    return targetm.gen_ccmp_next (tmp, rcode, op0, op1,
-					  get_rtx_code (code, 0));
+	  tmp = expand_ccmp_expr_1 (gs0, prep_seq, gen_seq);
+	  if (!tmp)
+	    return NULL_RTX;
+
+	  return expand_ccmp_next (gs1, code, tmp, prep_seq, gen_seq);
 	}
       else
 	{
@@ -264,23 +261,30 @@ expand_ccmp_expr (gimple g)
 {
   rtx_insn *last;
   rtx tmp;
+  rtx prep_seq, gen_seq;
+
+  prep_seq = gen_seq = NULL_RTX;
 
   if (!ccmp_candidate_p (g))
     return NULL_RTX;
 
   last = get_last_insn ();
-  tmp = expand_ccmp_expr_1 (g);
+  tmp = expand_ccmp_expr_1 (g, &prep_seq, &gen_seq);
 
   if (tmp)
     {
       enum insn_code icode;
       enum machine_mode cc_mode = CCmode;
-
       tree lhs = gimple_assign_lhs (g);
+
       /* TMP should be CC.  If it is used in a GIMPLE_COND, just return it.
 	 Note: Target needs to define "cbranchcc4".  */
       if (used_in_cond_stmt_p (lhs))
-	return tmp;
+	{
+	  emit_insn (prep_seq);
+	  emit_insn (gen_seq);
+	  return tmp;
+	}
 
 #ifdef SELECT_CC_MODE
       cc_mode = SELECT_CC_MODE (NE, tmp, const0_rtx);
@@ -290,9 +294,12 @@ expand_ccmp_expr (gimple g)
       icode = optab_handler (cstore_optab, cc_mode);
       if (icode != CODE_FOR_nothing)
 	{
-	  tree lhs = gimple_assign_lhs (g);
 	  enum machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
 	  rtx target = gen_reg_rtx (mode);
+
+	  emit_insn (prep_seq);
+	  emit_insn (gen_seq);
+
 	  tmp = emit_cstore (target, icode, NE, cc_mode, cc_mode,
 			     0, tmp, const0_rtx, 1, mode);
 	  if (tmp)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 226a808..9acb892 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -10283,6 +10283,198 @@ aarch64_use_by_pieces_infrastructure_p (unsigned HOST_WIDE_INT size,
   return default_use_by_pieces_infrastructure_p (size, align, op, speed_p);
 }
 
+static enum machine_mode
+aarch64_code_to_ccmode (enum rtx_code code)
+{
+  switch (code)
+    {
+    case NE:
+      return CC_DNEmode;
+
+    case EQ:
+      return CC_DEQmode;
+
+    case LE:
+      return CC_DLEmode;
+
+    case LT:
+      return CC_DLTmode;
+
+    case GE:
+      return CC_DGEmode;
+
+    case GT:
+      return CC_DGTmode;
+
+    case LEU:
+      return CC_DLEUmode;
+
+    case LTU:
+      return CC_DLTUmode;
+
+    case GEU:
+      return CC_DGEUmode;
+
+    case GTU:
+      return CC_DGTUmode;
+
+    default:
+      return CCmode;
+    }
+}
+
+static rtx
+aarch64_gen_ccmp_first (rtx *prep_seq, rtx *gen_seq,
+			int code, tree treeop0, tree treeop1)
+{
+  enum machine_mode op_mode, cmp_mode, cc_mode;
+  rtx op0, op1, cmp, target;
+  int unsignedp = TYPE_UNSIGNED (TREE_TYPE (treeop0));
+  enum insn_code icode;
+  struct expand_operand ops[4];
+
+  cc_mode = aarch64_code_to_ccmode ((enum rtx_code) code);
+  if (cc_mode == CCmode)
+    return NULL_RTX;
+
+  start_sequence ();
+  expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1, EXPAND_NORMAL);
+
+  op_mode = GET_MODE (op0);
+  if (op_mode == VOIDmode)
+    op_mode = GET_MODE (op1);
+
+  switch (op_mode)
+    {
+    case QImode:
+    case HImode:
+    case SImode:
+      cmp_mode = SImode;
+      icode = CODE_FOR_cmpsi;
+      break;
+
+    case DImode:
+      cmp_mode = DImode;
+      icode = CODE_FOR_cmpdi;
+      break;
+
+    default:
+      end_sequence ();
+      return NULL_RTX;
+    }
+
+  op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode, unsignedp);
+  op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode, unsignedp);
+  if (!op0 || !op1)
+    {
+      end_sequence ();
+      return NULL_RTX;
+    }
+  *prep_seq = get_insns ();
+  end_sequence ();
+
+  cmp = gen_rtx_fmt_ee ((enum rtx_code) code, cmp_mode, op0, op1);
+  target = gen_rtx_REG (CCmode, CC_REGNUM);
+
+  create_output_operand (&ops[0], target, CCmode);
+  create_fixed_operand (&ops[1], cmp);
+  create_fixed_operand (&ops[2], op0);
+  create_fixed_operand (&ops[3], op1);
+
+  start_sequence ();
+  if (!maybe_expand_insn (icode, 4, ops))
+    {
+      end_sequence ();
+      return NULL_RTX;
+    }
+  *gen_seq = get_insns ();
+  end_sequence ();
+
+  return gen_rtx_REG (cc_mode, CC_REGNUM);
+}
+
+static rtx
+aarch64_gen_ccmp_next (rtx *prep_seq, rtx *gen_seq, rtx prev, int cmp_code,
+		       tree treeop0, tree treeop1, int bit_code)
+{
+  rtx op0, op1, cmp0, cmp1, target;
+  enum machine_mode op_mode, cmp_mode, cc_mode;
+  int unsignedp = TYPE_UNSIGNED (TREE_TYPE (treeop0));
+  enum insn_code icode = CODE_FOR_ccmp_andsi;
+  struct expand_operand ops[6];
+
+  cc_mode = aarch64_code_to_ccmode ((enum rtx_code) cmp_code);
+  if (cc_mode == CCmode)
+    return NULL_RTX;
+
+  push_to_sequence ((rtx_insn*) *prep_seq);
+  expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1, EXPAND_NORMAL);
+
+  op_mode = GET_MODE (op0);
+  if (op_mode == VOIDmode)
+    op_mode = GET_MODE (op1);
+
+  switch (op_mode)
+    {
+    case QImode:
+    case HImode:
+    case SImode:
+      cmp_mode = SImode;
+      icode = (enum rtx_code) bit_code == AND ? CODE_FOR_ccmp_andsi
+						: CODE_FOR_ccmp_iorsi;
+      break;
+
+    case DImode:
+      cmp_mode = DImode;
+      icode = (enum rtx_code) bit_code == AND ? CODE_FOR_ccmp_anddi
+						: CODE_FOR_ccmp_iordi;
+      break;
+
+    default:
+      end_sequence ();
+      return NULL_RTX;
+    }
+
+  op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode, unsignedp);
+  op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode, unsignedp);
+  if (!op0 || !op1)
+    {
+      end_sequence ();
+      return NULL_RTX;
+    }
+  *prep_seq = get_insns ();
+  end_sequence ();
+
+  target = gen_rtx_REG (cc_mode, CC_REGNUM);
+  cmp1 = gen_rtx_fmt_ee ((enum rtx_code) cmp_code, cmp_mode, op0, op1);
+  cmp0 = gen_rtx_fmt_ee (NE, cmp_mode, prev, const0_rtx);
+
+  create_fixed_operand (&ops[0], prev);
+  create_fixed_operand (&ops[1], target);
+  create_fixed_operand (&ops[2], op0);
+  create_fixed_operand (&ops[3], op1);
+  create_fixed_operand (&ops[4], cmp0);
+  create_fixed_operand (&ops[5], cmp1);
+
+  push_to_sequence ((rtx_insn*) *gen_seq);
+  if (!maybe_expand_insn (icode, 6, ops))
+    {
+      end_sequence ();
+      return NULL_RTX;
+    }
+
+  *gen_seq = get_insns ();
+  end_sequence ();
+
+  return target;
+}
+
+#undef TARGET_GEN_CCMP_FIRST
+#define TARGET_GEN_CCMP_FIRST aarch64_gen_ccmp_first
+
+#undef TARGET_GEN_CCMP_NEXT
+#define TARGET_GEN_CCMP_NEXT aarch64_gen_ccmp_next
+
 /* Implement TARGET_SCHED_MACRO_FUSION_P.  Return true if target supports
    instruction fusion of some sort.  */
 
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 97d7009..0aa03c7 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -257,7 +257,7 @@
   ""
   "")
 
-(define_insn "*ccmp_and"
+(define_insn "ccmp_and<mode>"
   [(set (match_operand 1 "ccmp_cc_register" "")
 	(compare
 	 (and:SI
@@ -276,7 +276,7 @@
   [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
 )
 
-(define_insn "*ccmp_ior"
+(define_insn "ccmp_ior<mode>"
   [(set (match_operand 1 "ccmp_cc_register" "")
 	(compare
 	 (ior:SI
@@ -295,6 +295,20 @@
   [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
 )
 
+(define_expand "cmp<mode>"
+  [(set (match_operand 0 "cc_register" "")
+        (match_operator:CC 1 "aarch64_comparison_operator"
+         [(match_operand:GPI 2 "register_operand" "")
+          (match_operand:GPI 3 "aarch64_plus_operand" "")]))]
+  ""
+  "
+  operands[1] = gen_rtx_fmt_ee (COMPARE, SELECT_CC_MODE (GET_CODE (operands[1]),
+							 operands[2],
+							 operands[3]),
+				 operands[2], operands[3]);
+  "
+)
+
 (define_insn "*condjump"
   [(set (pc) (if_then_else (match_operator 0 "aarch64_comparison_operator"
 			    [(match_operand 1 "cc_register" "") (const_int 0)])
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index ee741a9..5d530b7 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -11240,18 +11240,25 @@ This target hook is required only when the target has several different
 modes and they have different conditional execution capability, such as ARM.
 @end deftypefn
 
-@deftypefn {Target Hook} rtx TARGET_GEN_CCMP_FIRST (int @var{code}, rtx @var{op0}, rtx @var{op1})
-This function emits a comparison insn for the first of a sequence of
- conditional comparisions.  It returns a comparison expression appropriate
- for passing to @code{gen_ccmp_next} or @code{cbranch_optab}.  @var{code} is
+@deftypefn {Target Hook} rtx TARGET_GEN_CCMP_FIRST (rtx *@var{prep_seq}, rtx *@var{gen_seq}, int @var{code}, tree @var{op0}, tree @var{op1})
+This function prepares to emit a comparison insn for the first compare in a
+ sequence of conditional comparisions.  It returns a appropriate @code{CC}
+ for passing to @code{gen_ccmp_next} or @code{cbranch_optab}.  The insns to
+ prepare the compare are saved in @var{prep_seq} and the compare insns are
+ saved in @var{gen_seq}.  They will be emitted when all the compares in the
+ the conditional comparision are generated without error.  @var{code} is
  the @code{rtx_code} of the compare for @var{op0} and @var{op1}.
 @end deftypefn
 
-@deftypefn {Target Hook} rtx TARGET_GEN_CCMP_NEXT (rtx @var{prev}, int @var{cmp_code}, rtx @var{op0}, rtx @var{op1}, int @var{bit_code})
-This function emits a conditional comparison within a sequence of
- conditional comparisons.  The @var{prev} expression is the result of a
- prior call to @code{gen_ccmp_first} or @code{gen_ccmp_next}.  It may return
- @code{NULL} if the combination of @var{prev} and this comparison is
+@deftypefn {Target Hook} rtx TARGET_GEN_CCMP_NEXT (rtx *@var{prep_seq}, rtx *@var{gen_seq}, rtx @var{prev}, int @var{cmp_code}, tree @var{op0}, tree @var{op1}, int @var{bit_code})
+This function prepare to emit a conditional comparison within a sequence of
+ conditional comparisons.  It returns a appropriate @code{CC} for passing to
+ @code{gen_ccmp_next} or @code{cbranch_optab}.  The insns to prepare the
+ compare are saved in @var{prep_seq} and the compare insns are saved in
+ @var{gen_seq}.  They will be emitted when all the compares in the conditional
+ comparision are generated without error.  The @var{prev} expression is the
+ result of a prior call to @code{gen_ccmp_first} or @code{gen_ccmp_next}.  It
+ may return @code{NULL} if the combination of @var{prev} and this comparison is
  not supported, otherwise the result must be appropriate for passing to
  @code{gen_ccmp_next} or @code{cbranch_optab}.  @var{code} is the
  @code{rtx_code} of the compare for @var{op0} and @var{op1}.  @var{bit_code}
diff --git a/gcc/target.def b/gcc/target.def
index e7cec46..271e3b7 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -2542,24 +2542,31 @@ modes and they have different conditional execution capability, such as ARM.",
 
 DEFHOOK
 (gen_ccmp_first,
- "This function emits a comparison insn for the first of a sequence of\n\
- conditional comparisions.  It returns a comparison expression appropriate\n\
- for passing to @code{gen_ccmp_next} or @code{cbranch_optab}.  @var{code} is\n\
+ "This function prepares to emit a comparison insn for the first compare in a\n\
+ sequence of conditional comparisions.  It returns a appropriate @code{CC}\n\
+ for passing to @code{gen_ccmp_next} or @code{cbranch_optab}.  The insns to\n\
+ prepare the compare are saved in @var{prep_seq} and the compare insns are\n\
+ saved in @var{gen_seq}.  They will be emitted when all the compares in the\n\
+ the conditional comparision are generated without error.  @var{code} is\n\
  the @code{rtx_code} of the compare for @var{op0} and @var{op1}.",
- rtx, (int code, rtx op0, rtx op1),
+ rtx, (rtx *prep_seq, rtx *gen_seq, int code, tree op0, tree op1),
  NULL)
 
 DEFHOOK
 (gen_ccmp_next,
- "This function emits a conditional comparison within a sequence of\n\
- conditional comparisons.  The @var{prev} expression is the result of a\n\
- prior call to @code{gen_ccmp_first} or @code{gen_ccmp_next}.  It may return\n\
- @code{NULL} if the combination of @var{prev} and this comparison is\n\
+ "This function prepare to emit a conditional comparison within a sequence of\n\
+ conditional comparisons.  It returns a appropriate @code{CC} for passing to\n\
+ @code{gen_ccmp_next} or @code{cbranch_optab}.  The insns to prepare the\n\
+ compare are saved in @var{prep_seq} and the compare insns are saved in\n\
+ @var{gen_seq}.  They will be emitted when all the compares in the conditional\n\
+ comparision are generated without error.  The @var{prev} expression is the\n\
+ result of a prior call to @code{gen_ccmp_first} or @code{gen_ccmp_next}.  It\n\
+ may return @code{NULL} if the combination of @var{prev} and this comparison is\n\
  not supported, otherwise the result must be appropriate for passing to\n\
  @code{gen_ccmp_next} or @code{cbranch_optab}.  @var{code} is the\n\
  @code{rtx_code} of the compare for @var{op0} and @var{op1}.  @var{bit_code}\n\
  is @code{AND} or @code{IOR}, which is the op on the two compares.",
- rtx, (rtx prev, int cmp_code, rtx op0, rtx op1, int bit_code),
+ rtx, (rtx *prep_seq, rtx *gen_seq, rtx prev, int cmp_code, tree op0, tree op1, int bit_code),
  NULL)
 
 /* Return a new value for loop unroll size.  */
diff --git a/gcc/testsuite/gcc.dg/pr64015.c b/gcc/testsuite/gcc.dg/pr64015.c
new file mode 100644
index 0000000..daf8393
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr64015.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 " } */
+
+int
+test (unsigned short a, unsigned char b)
+{
+  return a > 0xfff2 && b > 252;
+}
+
+/* { dg-final { scan-assembler "ccmp" { target aarch64*-*-* } } } */

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

[Jiong Wang]

On 15/12/14 08:41, Zhenqiang Chen wrote:

>
>> -----Original Message-----
>> From: Richard Henderson [mailto:rth@redhat.com]
>> Sent: Saturday, December 13, 2014 3:26 AM
>> To: Zhenqiang Chen
>> Cc: Marcus Shawcroft; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH, AARCH64] Fix ICE in CCMP (PR64015)
>>
>>> -	  tree lhs = gimple_assign_lhs (g);
>>>   	  enum machine_mode mode = TYPE_MODE (TREE_TYPE (lhs));
>>>   	  rtx target = gen_reg_rtx (mode);
>>> +
>>> +	  start_sequence ();
>>>   	  tmp = emit_cstore (target, icode, NE, cc_mode, cc_mode,
>>>   			     0, tmp, const0_rtx, 1, mode);
>>>   	  if (tmp)
>>> -	    return tmp;
>>> +	    {
>>> +	      rtx seq = get_insns ();
>>> +	      end_sequence ();
>>> +	      emit_insn (prep_seq);
>>> +	      emit_insn (gen_seq);
>>> +	      emit_insn (seq);
>>> +	      return tmp;
>>> +	    }
>>> +	  end_sequence ();
>> Given that you're already doing delete_insns_since (last) at the end of this
>> function, I don't think you need a new sequence around the emit_cstore.
>> Just
>>
>> 	emit_insn (prep_seq);
>> 	emit_insn (gen_seq);
>> 	tmp = emit_cstore (...);
>> 	if (tmp)
>> 	  return tmp;
> Updated.
>   
>>> +  int unsignedp = code == LTU || code == LEU || code == GTU || code
>>> + == GEU;
>> You don't need to examine the code, you can look at the argument:
>>
>>    TYPE_UNSIGNED (TREE_TYPE (treeop0))
>   
> Updated.
>   
>>> +  op0 = prepare_operand (icode, op0, 2, op_mode, cmp_mode,
>>> + unsignedp);
>>> +  op1 = prepare_operand (icode, op1, 3, op_mode, cmp_mode,
>>> + unsignedp);  if (!op0 || !op1)
>>> +    {
>>> +      end_sequence ();
>>> +      return NULL_RTX;
>>> +    }
>>> +  *prep_seq = get_insns ();
>>> +  end_sequence ();
>>> +
>>> +  cmp = gen_rtx_fmt_ee ((enum rtx_code) code, cmp_mode, op0, op1);
>>> + target = gen_rtx_REG (CCmode, CC_REGNUM);
>>> +
>>> +  create_output_operand (&ops[0], target, CCmode);
>>> + create_fixed_operand (&ops[1], cmp);  create_fixed_operand (&ops[2],
>>> + op0);  create_fixed_operand (&ops[3], op1);
>> Hmm.  With so many fixed operands, I think you may be better off not
>> creating the cmp<mode> expander in the first place.  Just inline the
>> SELECT_CC_MODE and everything right here.
> In the patch, I use prepare_operand (icode, op0, 2, ...) to do the operand MODE conversion (from HI/QI to SI), which needs a cmp<mode> expander. Without it, I have to add additional codes to do the conversion (as it in previous patch, which leads to PR64015).

Ping~,

verified this patch will pass speck2k6 build without ICE anymore.

>
> Thanks!
> -Zhenqiang

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

[Richard Henderson]

On 12/15/2014 12:41 AM, Zhenqiang Chen wrote:
> +(define_expand "cmp<mode>"
> +  [(set (match_operand 0 "cc_register" "")
> +        (match_operator:CC 1 "aarch64_comparison_operator"
> +         [(match_operand:GPI 2 "register_operand" "")
> +          (match_operand:GPI 3 "aarch64_plus_operand" "")]))]
> +  ""
> +  "
> +  operands[1] = gen_rtx_fmt_ee (COMPARE, SELECT_CC_MODE (GET_CODE (operands[1]),
> +							 operands[2],
> +							 operands[3]),
> +				 operands[2], operands[3]);
> +  "
> +)

Use { } not "" for the C portion.

Otherwise ok.


r~

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

[Marcus Shawcroft]

On 15 January 2015 at 18:18, Richard Henderson <rth@redhat.com> wrote:
> On 12/15/2014 12:41 AM, Zhenqiang Chen wrote:
>> +(define_expand "cmp<mode>"
>> +  [(set (match_operand 0 "cc_register" "")
>> +        (match_operator:CC 1 "aarch64_comparison_operator"
>> +         [(match_operand:GPI 2 "register_operand" "")
>> +          (match_operand:GPI 3 "aarch64_plus_operand" "")]))]
>> +  ""
>> +  "
>> +  operands[1] = gen_rtx_fmt_ee (COMPARE, SELECT_CC_MODE (GET_CODE (operands[1]),
>> +                                                      operands[2],
>> +                                                      operands[3]),
>> +                              operands[2], operands[3]);
>> +  "
>> +)
>
> Use { } not "" for the C portion.
>
> Otherwise ok.

Jiong... this is fine with me. /Marcus

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

[Christophe Lyon]

On 16 January 2015 at 11:54, Marcus Shawcroft
<marcus.shawcroft@gmail.com> wrote:
> On 15 January 2015 at 18:18, Richard Henderson <rth@redhat.com> wrote:
>> On 12/15/2014 12:41 AM, Zhenqiang Chen wrote:
>>> +(define_expand "cmp<mode>"
>>> +  [(set (match_operand 0 "cc_register" "")
>>> +        (match_operator:CC 1 "aarch64_comparison_operator"
>>> +         [(match_operand:GPI 2 "register_operand" "")
>>> +          (match_operand:GPI 3 "aarch64_plus_operand" "")]))]
>>> +  ""
>>> +  "
>>> +  operands[1] = gen_rtx_fmt_ee (COMPARE, SELECT_CC_MODE (GET_CODE (operands[1]),
>>> +                                                      operands[2],
>>> +                                                      operands[3]),
>>> +                              operands[2], operands[3]);
>>> +  "
>>> +)
>>
>> Use { } not "" for the C portion.
>>
>> Otherwise ok.
>
> Jiong... this is fine with me. /Marcus

Jiong,

I have noticed regressions on aarch64 after this patch:

See: http://abe.tcwglab.linaro.org/logs/validations/cross-validation/gcc/trunk/219723/report-build-info.html
Passed now fails          [PASS => FAIL]:
  gcc.target/aarch64/test_frame_1.c scan-assembler-times ldr\tx30,
\\[sp\\], [0-9]+ 3
  gcc.target/aarch64/test_frame_2.c scan-assembler-times ldp\tx19,
x30, \\[sp\\], [0-9]+ 2
  gcc.target/aarch64/test_frame_4.c scan-assembler-times ldp\tx19,
x30, \\[sp\\], [0-9]+ 2
  gcc.target/aarch64/test_frame_6.c scan-assembler-times ldr\tx30,
\\[sp\\], [0-9]+ 3
  gcc.target/aarch64/test_frame_7.c scan-assembler-times ldp\tx19,
x30, \\[sp\\], [0-9]+ 2

Is this expected?

Thanks,

Christophe.

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

[Andrew Pinski]

On Sun, Jan 18, 2015 at 11:58 AM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 16 January 2015 at 11:54, Marcus Shawcroft
> <marcus.shawcroft@gmail.com> wrote:
>> On 15 January 2015 at 18:18, Richard Henderson <rth@redhat.com> wrote:
>>> On 12/15/2014 12:41 AM, Zhenqiang Chen wrote:
>>>> +(define_expand "cmp<mode>"
>>>> +  [(set (match_operand 0 "cc_register" "")
>>>> +        (match_operator:CC 1 "aarch64_comparison_operator"
>>>> +         [(match_operand:GPI 2 "register_operand" "")
>>>> +          (match_operand:GPI 3 "aarch64_plus_operand" "")]))]
>>>> +  ""
>>>> +  "
>>>> +  operands[1] = gen_rtx_fmt_ee (COMPARE, SELECT_CC_MODE (GET_CODE (operands[1]),
>>>> +                                                      operands[2],
>>>> +                                                      operands[3]),
>>>> +                              operands[2], operands[3]);
>>>> +  "
>>>> +)
>>>
>>> Use { } not "" for the C portion.
>>>
>>> Otherwise ok.
>>
>> Jiong... this is fine with me. /Marcus
>
> Jiong,
>
> I have noticed regressions on aarch64 after this patch:
>
> See: http://abe.tcwglab.linaro.org/logs/validations/cross-validation/gcc/trunk/219723/report-build-info.html
> Passed now fails          [PASS => FAIL]:
>   gcc.target/aarch64/test_frame_1.c scan-assembler-times ldr\tx30,
> \\[sp\\], [0-9]+ 3
>   gcc.target/aarch64/test_frame_2.c scan-assembler-times ldp\tx19,
> x30, \\[sp\\], [0-9]+ 2
>   gcc.target/aarch64/test_frame_4.c scan-assembler-times ldp\tx19,
> x30, \\[sp\\], [0-9]+ 2
>   gcc.target/aarch64/test_frame_6.c scan-assembler-times ldr\tx30,
> \\[sp\\], [0-9]+ 3
>   gcc.target/aarch64/test_frame_7.c scan-assembler-times ldp\tx19,
> x30, \\[sp\\], [0-9]+ 2
>
> Is this expected?

Yes and now you just have to revert the revert of my patch to fix those.

Thanks,
Andrew

>
> Thanks,
>
> Christophe.

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

[Christophe Lyon]

On 18 January 2015 at 21:22, Andrew Pinski <pinskia@gmail.com> wrote:
> On Sun, Jan 18, 2015 at 11:58 AM, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 16 January 2015 at 11:54, Marcus Shawcroft
>> <marcus.shawcroft@gmail.com> wrote:
>>> On 15 January 2015 at 18:18, Richard Henderson <rth@redhat.com> wrote:
>>>> On 12/15/2014 12:41 AM, Zhenqiang Chen wrote:
>>>>> +(define_expand "cmp<mode>"
>>>>> +  [(set (match_operand 0 "cc_register" "")
>>>>> +        (match_operator:CC 1 "aarch64_comparison_operator"
>>>>> +         [(match_operand:GPI 2 "register_operand" "")
>>>>> +          (match_operand:GPI 3 "aarch64_plus_operand" "")]))]
>>>>> +  ""
>>>>> +  "
>>>>> +  operands[1] = gen_rtx_fmt_ee (COMPARE, SELECT_CC_MODE (GET_CODE (operands[1]),
>>>>> +                                                      operands[2],
>>>>> +                                                      operands[3]),
>>>>> +                              operands[2], operands[3]);
>>>>> +  "
>>>>> +)
>>>>
>>>> Use { } not "" for the C portion.
>>>>
>>>> Otherwise ok.
>>>
>>> Jiong... this is fine with me. /Marcus
>>
>> Jiong,
>>
>> I have noticed regressions on aarch64 after this patch:
>>
>> See: http://abe.tcwglab.linaro.org/logs/validations/cross-validation/gcc/trunk/219723/report-build-info.html
>> Passed now fails          [PASS => FAIL]:
>>   gcc.target/aarch64/test_frame_1.c scan-assembler-times ldr\tx30,
>> \\[sp\\], [0-9]+ 3
>>   gcc.target/aarch64/test_frame_2.c scan-assembler-times ldp\tx19,
>> x30, \\[sp\\], [0-9]+ 2
>>   gcc.target/aarch64/test_frame_4.c scan-assembler-times ldp\tx19,
>> x30, \\[sp\\], [0-9]+ 2
>>   gcc.target/aarch64/test_frame_6.c scan-assembler-times ldr\tx30,
>> \\[sp\\], [0-9]+ 3
>>   gcc.target/aarch64/test_frame_7.c scan-assembler-times ldp\tx19,
>> x30, \\[sp\\], [0-9]+ 2
>>
>> Is this expected?
>
> Yes and now you just have to revert the revert of my patch to fix those.
>
Thanks for the clarification/confirmation, I thought I had seen
something like that but I was confused.

> Thanks,
> Andrew
>
>>
>> Thanks,
>>
>> Christophe.