[PATCH, i386]: Do not emit x87 FP reg-stack compensation pops from output_fp_compare

Uros Bizjak ubizjak@gmail.com
Tue Oct 17 16:50:00 GMT 2017


Hello!

Currently, x87 FP stack compensation pops for FTST and FCOMIP
instructions are emitted from output_fp_compare function as an
assembly code. Attached patch moves detection of these two
instructions to reg-stack.c and handles compensation pops during
reg-stack processing. This change further allows for a massive cleanup
in output_fp_compare.

2017-10-17  Uros Bizjak  <ubizjak@gmail.com>

    * reg-stack.c (compare_for_stack_reg): Add bool argument.
    Detect FTST instruction and handle its register pops.  Only pop
    second operand if can_pop_second_op is true.
    (subst_stack_regs_pat) <case COMPARE>: Detect FCOMI instruction to
    set can_pop_second_op to false in the compare_for_stack_reg call.

    * config/i386/i386.md (*cmpi<FPCMP:unord><MODEF:mode>): Only call
    output_fp_compare for stack register operands.
    * config/i386/i386.c (output_fp_compare): Do not output SSE compare
    instructions here.  Do not emit stack register pops here.  Assert
    that FCOMPP pops next to top stack register.  Rewrite function.

Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline SVN.

Uros.
-------------- next part --------------
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 253812)
+++ config/i386/i386.c	(working copy)
@@ -18879,120 +18879,65 @@ output_387_ffreep (rtx *operands ATTRIBUTE_UNUSED,
    should be used.  UNORDERED_P is true when fucom should be used.  */
 
 const char *
-output_fp_compare (rtx_insn *insn, rtx *operands, bool eflags_p, bool unordered_p)
+output_fp_compare (rtx_insn *insn, rtx *operands,
+		   bool eflags_p, bool unordered_p)
 {
-  int stack_top_dies;
-  rtx cmp_op0, cmp_op1;
-  int is_sse = SSE_REG_P (operands[0]) || SSE_REG_P (operands[1]);
+  rtx *xops = eflags_p ? &operands[0] : &operands[1];
+  bool stack_top_dies;
 
+  static char buf[40];
+  const char *p, *r;
+ 
+  gcc_assert (STACK_TOP_P (xops[0]));
+
+  stack_top_dies = find_regno_note (insn, REG_DEAD, FIRST_STACK_REG);
+
   if (eflags_p)
     {
-      cmp_op0 = operands[0];
-      cmp_op1 = operands[1];
+      p = unordered_p ? "fucomi" : "fcomi";
+      strcpy (buf, p);
+
+      r = "p\t{%y1, %0|%0, %y1}";
+      strcat (buf, r + !stack_top_dies);
+
+      return buf;
     }
-  else
+
+  if (STACK_REG_P (xops[1])
+      && stack_top_dies
+      && find_regno_note (insn, REG_DEAD, FIRST_STACK_REG + 1))
     {
-      cmp_op0 = operands[1];
-      cmp_op1 = operands[2];
+      gcc_assert (REGNO (xops[1]) == FIRST_STACK_REG + 1);
+
+      /* If both the top of the 387 stack die, and the other operand
+	 is also a stack register that dies, then this must be a
+	 `fcompp' float compare.  */
+      p = unordered_p ? "fucompp" : "fcompp";
+      strcpy (buf, p);
     }
-
-  if (is_sse)
+  else if (const0_operand (xops[1], VOIDmode))
     {
-      if (GET_MODE (operands[0]) == SFmode)
-	if (unordered_p)
-	  return "%vucomiss\t{%1, %0|%0, %1}";
-	else
-	  return "%vcomiss\t{%1, %0|%0, %1}";
-      else
-	if (unordered_p)
-	  return "%vucomisd\t{%1, %0|%0, %1}";
-	else
-	  return "%vcomisd\t{%1, %0|%0, %1}";
+      gcc_assert (!unordered_p);
+      strcpy (buf, "ftst");
     }
-
-  gcc_assert (STACK_TOP_P (cmp_op0));
-
-  stack_top_dies = find_regno_note (insn, REG_DEAD, FIRST_STACK_REG) != 0;
-
-  if (cmp_op1 == CONST0_RTX (GET_MODE (cmp_op1)))
+  else
     {
-      if (stack_top_dies)
+      if (GET_MODE_CLASS (GET_MODE (xops[1])) == MODE_INT)
 	{
-	  output_asm_insn ("ftst\n\tfnstsw\t%0", operands);
-	  return output_387_ffreep (operands, 1);
+	  gcc_assert (!unordered_p);
+	  p = "ficom";
 	}
       else
-	return "ftst\n\tfnstsw\t%0";
-    }
+	p = unordered_p ? "fucom" : "fcom";
 
-  if (STACK_REG_P (cmp_op1)
-      && stack_top_dies
-      && find_regno_note (insn, REG_DEAD, REGNO (cmp_op1))
-      && REGNO (cmp_op1) != FIRST_STACK_REG)
-    {
-      /* If both the top of the 387 stack dies, and the other operand
-	 is also a stack register that dies, then this must be a
-	 `fcompp' float compare */
+      strcpy (buf, p);
 
-      if (eflags_p)
-	{
-	  /* There is no double popping fcomi variant.  Fortunately,
-	     eflags is immune from the fstp's cc clobbering.  */
-	  if (unordered_p)
-	    output_asm_insn ("fucomip\t{%y1, %0|%0, %y1}", operands);
-	  else
-	    output_asm_insn ("fcomip\t{%y1, %0|%0, %y1}", operands);
-	  return output_387_ffreep (operands, 0);
-	}
-      else
-	{
-	  if (unordered_p)
-	    return "fucompp\n\tfnstsw\t%0";
-	  else
-	    return "fcompp\n\tfnstsw\t%0";
-	}
+      r = "p%Z2\t%y2";
+      strcat (buf, r + !stack_top_dies);
     }
-  else
-    {
-      /* Encoded here as eflags_p | intmode | unordered_p | stack_top_dies.  */
 
-      static const char * const alt[16] =
-      {
-	"fcom%Z2\t%y2\n\tfnstsw\t%0",
-	"fcomp%Z2\t%y2\n\tfnstsw\t%0",
-	"fucom%Z2\t%y2\n\tfnstsw\t%0",
-	"fucomp%Z2\t%y2\n\tfnstsw\t%0",
-
-	"ficom%Z2\t%y2\n\tfnstsw\t%0",
-	"ficomp%Z2\t%y2\n\tfnstsw\t%0",
-	NULL,
-	NULL,
-
-	"fcomi\t{%y1, %0|%0, %y1}",
-	"fcomip\t{%y1, %0|%0, %y1}",
-	"fucomi\t{%y1, %0|%0, %y1}",
-	"fucomip\t{%y1, %0|%0, %y1}",
-
-	NULL,
-	NULL,
-	NULL,
-	NULL
-      };
-
-      int mask;
-      const char *ret;
-
-      mask  = eflags_p << 3;
-      mask |= (GET_MODE_CLASS (GET_MODE (cmp_op1)) == MODE_INT) << 2;
-      mask |= unordered_p << 1;
-      mask |= stack_top_dies;
-
-      gcc_assert (mask < 16);
-      ret = alt[mask];
-      gcc_assert (ret);
-
-      return ret;
-    }
+  output_asm_insn (buf, operands);
+  return "fnstsw\t%0";
 }
 
 void
Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revision 253812)
+++ config/i386/i386.md	(working copy)
@@ -1685,8 +1685,7 @@
    (set_attr "mode" "SI")])
 
 ;; Pentium Pro can do steps 1 through 3 in one go.
-;; comi*, ucomi*, fcomi*, ficomi*, fucomi*
-;; (these i387 instructions set flags directly)
+;; (these instructions set flags directly)
 
 (define_mode_iterator FPCMP [CCFP CCFPU])
 (define_mode_attr unord [(CCFP "") (CCFPU "u")])
@@ -1698,8 +1697,10 @@
 	  (match_operand:MODEF 1 "register_ssemem_operand" "f,vm")))]
   "(SSE_FLOAT_MODE_P (<MODEF:MODE>mode) && TARGET_SSE_MATH)
    || (TARGET_80387 && TARGET_CMOVE)"
-  "* return output_fp_compare (insn, operands, true,
-			       <FPCMP:MODE>mode == CCFPUmode);"
+  "@
+   * return output_fp_compare (insn, operands, true, \
+			       <FPCMP:MODE>mode == CCFPUmode);
+   %v<FPCMP:unord>comi<MODEF:ssemodesuffix>\t{%1, %0|%0, %1}"
   [(set_attr "type" "fcmp,ssecomi")
    (set_attr "prefix" "orig,maybe_vex")
    (set_attr "mode" "<MODEF:MODE>")
Index: reg-stack.c
===================================================================
--- reg-stack.c	(revision 253812)
+++ reg-stack.c	(working copy)
@@ -262,7 +262,7 @@ static bool move_for_stack_reg (rtx_insn *, stack_
 static bool move_nan_for_stack_reg (rtx_insn *, stack_ptr, rtx);
 static int swap_rtx_condition_1 (rtx);
 static int swap_rtx_condition (rtx_insn *);
-static void compare_for_stack_reg (rtx_insn *, stack_ptr, rtx);
+static void compare_for_stack_reg (rtx_insn *, stack_ptr, rtx, bool);
 static bool subst_stack_regs_pat (rtx_insn *, stack_ptr, rtx);
 static void subst_asm_stack_regs (rtx_insn *, stack_ptr);
 static bool subst_stack_regs (rtx_insn *, stack_ptr);
@@ -1325,7 +1325,8 @@ swap_rtx_condition (rtx_insn *insn)
    set up.  */
 
 static void
-compare_for_stack_reg (rtx_insn *insn, stack_ptr regstack, rtx pat_src)
+compare_for_stack_reg (rtx_insn *insn, stack_ptr regstack,
+		       rtx pat_src, bool can_pop_second_op)
 {
   rtx *src1, *src2;
   rtx src1_note, src2_note;
@@ -1366,8 +1367,18 @@ static void
 
   if (src1_note)
     {
-      pop_stack (regstack, REGNO (XEXP (src1_note, 0)));
-      replace_reg (&XEXP (src1_note, 0), FIRST_STACK_REG);
+      if (*src2 == CONST0_RTX (GET_MODE (*src2)))
+	{
+	  /* This is `ftst' insn that can't pop register.  */
+	  remove_regno_note (insn, REG_DEAD, REGNO (XEXP (src1_note, 0)));
+	  emit_pop_insn (insn, regstack, XEXP (src1_note, 0),
+			 EMIT_AFTER);
+	}
+      else
+	{
+	  pop_stack (regstack, REGNO (XEXP (src1_note, 0)));
+	  replace_reg (&XEXP (src1_note, 0), FIRST_STACK_REG);
+	}
     }
 
   /* If the second operand dies, handle that.  But if the operands are
@@ -1384,7 +1395,7 @@ static void
 	 at top (FIRST_STACK_REG) now.  */
 
       if (get_hard_regnum (regstack, XEXP (src2_note, 0)) == FIRST_STACK_REG
-	  && src1_note)
+	  && src1_note && can_pop_second_op)
 	{
 	  pop_stack (regstack, REGNO (XEXP (src2_note, 0)));
 	  replace_reg (&XEXP (src2_note, 0), FIRST_STACK_REG + 1);
@@ -1550,7 +1561,9 @@ subst_stack_regs_pat (rtx_insn *insn, stack_ptr re
 	switch (GET_CODE (pat_src))
 	  {
 	  case COMPARE:
-	    compare_for_stack_reg (insn, regstack, pat_src);
+	    /* `fcomi' insn can't pop two regs.  */
+	    compare_for_stack_reg (insn, regstack, pat_src,
+				   REGNO (*dest) != FLAGS_REG);
 	    break;
 
 	  case CALL:
@@ -1970,7 +1983,7 @@ subst_stack_regs_pat (rtx_insn *insn, stack_ptr re
 		pat_src = XVECEXP (pat_src, 0, 0);
 		gcc_assert (GET_CODE (pat_src) == COMPARE);
 
-		compare_for_stack_reg (insn, regstack, pat_src);
+		compare_for_stack_reg (insn, regstack, pat_src, true);
 		break;
 
 	      default:


More information about the Gcc-patches mailing list