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: 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: gen-ccmp-v2.patch
Description: Binary data