This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]