[MIPS, committed] Fix LO liveness in MIPS16 divmod patterns

Richard Sandiford rdsandiford@googlemail.com
Sat Jan 25 20:16:00 GMT 2014


This patch fixes a bug I introduced with:

2013-05-25  Richard Sandiford  <rdsandiford@googlemail.com>

	PR target/53916
	* config/mips/constraints.md (kl): New constraint.
	* config/mips/mips.md (divmod<mode>4, udivmod<mode>4): Delete.
	(divmod<mode>4_internal): Rename to divmod<mode>4.  Use "kl" as the
	constraint for operand 0.  Split after CSE for MIPS16.  Emit a move
	from LO for MIPS16.
	(udivmod<mode>4_internal): Likewise udivmod<mode>4.

The idea was to defer making the LO target of the division explicit
until cse_not_expected, so that we can still CSE two divmods, one in
which the division is used and one in which the modulus is used.
The problem was that the pre-CSE pattern didn't mention LO at all,
so an existing definition and use of LO could be split across the
division.  This caused a miscompilation of gfortran.dg/transpose_2.f90
and gfortran.dg/typebound_operator_9.f03.

Pretty lame bug, sorry.  Warning bells should have been ringing
at the use of gen_rtx_REG in a split.

Tested on mips64-linux-gnu, where it fixes the Fortran failures.
Applied to trunk.  The original 53916 patch only went in after 4.8
so no backport is needed.

Thanks,
Richard


gcc/
	* config/mips/constraints.md (kl): Delete.
	* config/mips/mips.md (divmod<mode>4, udivmod<mode>4): Turn into
	define expands, using...
	(divmod<mode>4_mips16, udivmod<mode>4_mips16): ...these new
	instructions for MIPS16.
	(*divmod<mode>4, *udivmod<mode>4): New patterns, taken from the
	non-MIPS16 version of the old divmod<mode>4 and udivmod<mode>4.

Index: gcc/config/mips/constraints.md
===================================================================
--- gcc/config/mips/constraints.md	2014-01-25 19:55:25.619282270 +0000
+++ gcc/config/mips/constraints.md	2014-01-25 20:10:58.866144137 +0000
@@ -92,12 +92,6 @@ (define_register_constraint "D" "COP3_RE
 ;; but the DSP version allows any accumulator target.
 (define_register_constraint "ka" "ISA_HAS_DSP_MULT ? ACC_REGS : MD_REGS")
 
-;; The register class to use for an allocatable division result.
-;; MIPS16 uses M16_REGS because LO is fixed.
-(define_register_constraint "kl"
-  "TARGET_MIPS16 ? M16_REGS : TARGET_BIG_ENDIAN ? MD1_REG : MD0_REG"
-  "@internal")
-
 (define_constraint "kf"
   "@internal"
   (match_operand 0 "force_to_mem_operand"))
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	2014-01-25 19:55:25.619282270 +0000
+++ gcc/config/mips/mips.md	2014-01-25 20:10:58.868144152 +0000
@@ -2559,56 +2559,129 @@ (define_insn "*recip<mode>3"
 
 ;; VR4120 errata MD(A1): signed division instructions do not work correctly
 ;; with negative operands.  We use special libgcc functions instead.
-;;
+(define_expand "divmod<mode>4"
+  [(parallel
+     [(set (match_operand:GPR 0 "register_operand")
+	   (div:GPR (match_operand:GPR 1 "register_operand")
+		    (match_operand:GPR 2 "register_operand")))
+      (set (match_operand:GPR 3 "register_operand")
+	   (mod:GPR (match_dup 1)
+		    (match_dup 2)))])]
+  "ISA_HAS_<D>DIV && !TARGET_FIX_VR4120"
+{
+  if (TARGET_MIPS16)
+    {
+      rtx lo = gen_rtx_REG (<MODE>mode, LO_REGNUM);
+      emit_insn (gen_divmod<mode>4_mips16 (operands[0], operands[1],
+					   operands[2], operands[3], lo));
+      DONE;
+    }
+})
+
+(define_insn_and_split "*divmod<mode>4"
+  [(set (match_operand:GPR 0 "register_operand" "=l")
+	(div:GPR (match_operand:GPR 1 "register_operand" "d")
+		 (match_operand:GPR 2 "register_operand" "d")))
+   (set (match_operand:GPR 3 "register_operand" "=d")
+	(mod:GPR (match_dup 1)
+		 (match_dup 2)))]
+  "ISA_HAS_<D>DIV && !TARGET_FIX_VR4120 && !TARGET_MIPS16"
+  "#"
+  "&& reload_completed"
+  [(const_int 0)]
+{
+  emit_insn (gen_divmod<mode>4_split (operands[3], operands[1], operands[2]));
+  DONE;
+}
+ [(set_attr "type" "idiv")
+  (set_attr "mode" "<MODE>")
+  (set_attr "insn_count" "2")])
+
 ;; Expand generates divmod instructions for individual division and modulus
 ;; operations.  We then rely on CSE to reuse earlier divmods where possible.
 ;; This means that, when generating MIPS16 code, it is better not to expose
 ;; the fixed LO register until after CSE has finished.  However, it's still
 ;; better to split before register allocation, so that we don't allocate
 ;; one of the scarce MIPS16 registers to an unused result.
-(define_insn_and_split "divmod<mode>4"
-  [(set (match_operand:GPR 0 "register_operand" "=kl")
+(define_insn_and_split "divmod<mode>4_mips16"
+  [(set (match_operand:GPR 0 "register_operand" "=d")
 	(div:GPR (match_operand:GPR 1 "register_operand" "d")
 		 (match_operand:GPR 2 "register_operand" "d")))
    (set (match_operand:GPR 3 "register_operand" "=d")
 	(mod:GPR (match_dup 1)
-		 (match_dup 2)))]
-  "ISA_HAS_<D>DIV && !TARGET_FIX_VR4120"
+		 (match_dup 2)))
+   (clobber (match_operand:GPR 4 "lo_operand" "=l"))]
+  "ISA_HAS_<D>DIV && !TARGET_FIX_VR4120 && TARGET_MIPS16"
   "#"
-  "&& ((TARGET_MIPS16 && cse_not_expected) || reload_completed)"
+  "&& cse_not_expected"
   [(const_int 0)]
 {
   emit_insn (gen_divmod<mode>4_split (operands[3], operands[1], operands[2]));
-  if (TARGET_MIPS16)
-    emit_move_insn (operands[0], gen_rtx_REG (<MODE>mode, LO_REGNUM));
+  emit_move_insn (operands[0], operands[4]);
   DONE;
 }
  [(set_attr "type" "idiv")
   (set_attr "mode" "<MODE>")
-  ;; Worst case for MIPS16.
   (set_attr "insn_count" "3")])
 
-;; See the comment above "divmod<mode>4" for the MIPS16 handling.
-(define_insn_and_split "udivmod<mode>4"
-  [(set (match_operand:GPR 0 "register_operand" "=kl")
+(define_expand "udivmod<mode>4"
+  [(parallel
+     [(set (match_operand:GPR 0 "register_operand")
+	   (udiv:GPR (match_operand:GPR 1 "register_operand")
+		     (match_operand:GPR 2 "register_operand")))
+      (set (match_operand:GPR 3 "register_operand")
+	   (umod:GPR (match_dup 1)
+		     (match_dup 2)))])]
+  "ISA_HAS_<D>DIV && !TARGET_FIX_VR4120"
+{
+  if (TARGET_MIPS16)
+    {
+      rtx lo = gen_rtx_REG (<MODE>mode, LO_REGNUM);
+      emit_insn (gen_udivmod<mode>4_mips16 (operands[0], operands[1],
+					    operands[2], operands[3], lo));
+      DONE;
+    }
+})
+
+(define_insn_and_split "*udivmod<mode>4"
+  [(set (match_operand:GPR 0 "register_operand" "=l")
 	(udiv:GPR (match_operand:GPR 1 "register_operand" "d")
 		  (match_operand:GPR 2 "register_operand" "d")))
    (set (match_operand:GPR 3 "register_operand" "=d")
 	(umod:GPR (match_dup 1)
 		  (match_dup 2)))]
-  "ISA_HAS_<D>DIV"
+  "ISA_HAS_<D>DIV && !TARGET_MIPS16"
   "#"
-  "(TARGET_MIPS16 && cse_not_expected) || reload_completed"
+  "reload_completed"
   [(const_int 0)]
 {
   emit_insn (gen_udivmod<mode>4_split (operands[3], operands[1], operands[2]));
-  if (TARGET_MIPS16)
-    emit_move_insn (operands[0], gen_rtx_REG (<MODE>mode, LO_REGNUM));
   DONE;
 }
   [(set_attr "type" "idiv")
    (set_attr "mode" "<MODE>")
-   ;; Worst case for MIPS16.
+   (set_attr "insn_count" "2")])
+
+;; See the comment above "divmod<mode>4_mips16" for the split timing.
+(define_insn_and_split "udivmod<mode>4_mips16"
+  [(set (match_operand:GPR 0 "register_operand" "=d")
+	(udiv:GPR (match_operand:GPR 1 "register_operand" "d")
+		  (match_operand:GPR 2 "register_operand" "d")))
+   (set (match_operand:GPR 3 "register_operand" "=d")
+	(umod:GPR (match_dup 1)
+		  (match_dup 2)))
+   (clobber (match_operand:GPR 4 "lo_operand" "=l"))]
+  "ISA_HAS_<D>DIV && TARGET_MIPS16"
+  "#"
+  "cse_not_expected"
+  [(const_int 0)]
+{
+  emit_insn (gen_udivmod<mode>4_split (operands[3], operands[1], operands[2]));
+  emit_move_insn (operands[0], operands[4]);
+  DONE;
+}
+  [(set_attr "type" "idiv")
+   (set_attr "mode" "<MODE>")
    (set_attr "insn_count" "3")])
 
 (define_expand "<u>divmod<mode>4_split"



More information about the Gcc-patches mailing list