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]

[ARM] movw fix


This patch fixes a bug exposed in 4.3 in a the execute/cmpdi-1.c testcase. Mainline doesn't fail, but I think that's because something else is preventing the sequence of events below from happening. I've been unable to generate a testcase that still triggers the bug, but that doesn't mean there is no bug to fix :)

The ARM md has the following two move instructions defined.

(define_insn "*arm_movw"
  [(set (match_operand:SI 0 "nonimmediate_operand" "=r")
	(high:SI (match_operand:SI 1 "general_operand"      "i")))]

(define_insn "*arm_movsi_insn"
  [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,r,r,rk,m")
	(match_operand:SI 1 "general_operand"      "rk, I,K,N,mi,rk"))]

consider the following pair of instructions:

(insn 91 89 92 2 cmpdi-1.i:190 (set (reg/f:SI 241)
  (high:SI (symbol_ref:SI ("*.LANCHOR0") [flags 0x182]))) 172 {*arm_movw}
   (expr_list:REG_EQUIV (high:SI (symbol_ref:SI ("*.LANCHOR0") [flags 0x182]))
        (nil)))

(insn 269 95 270 2 cmpdi-1.i:193 (set (reg/f:SI 245)
        (reg/f:SI 241)) 173 {*arm_movsi_insn} (nil))

In this case 269 was created by the loop optimizer as an invariant, but that's not important right now. If register pressure is high, reload will rematerialize the equivalent value directly into whatever register it assigns pseudo 245 to. In this case we get:

(insn 269 95 286 2 cmpdi-1.i:193 (set (reg:SI 3 r3)
  (high:SI (symbol_ref:SI ("*.LANCHOR0") [flags 0x182])))
   173 {*arm_movsi_insn} (nil))

That looks fine, but notice that it is *arm_movsi_insn not *arm_movw. IIUC reload does not expect the act of picking reload operands to change which instruction a pattern matches. general_operand permits (high:SI ...), which explains why reload was happy rematerializing the (high:SI (symbol_ref ...)) into r3. So we have two rtl instructions that match the same RTL pattern, and it is just dumb luck that *arm_movw matches nearly all the cases it needs to -- I'll bet if the two define_insns are in the opposite order in the MD file, that'll no longer be true.

This patch removes the *arm_movw instruction, and alters the *arm_movsi_insn appropriately. We need a new predicate that accepts (in TARGET32 mode) a HIGH something, and removes that condition from the current N constraint. Then we teach arm_print_operand about printing a HIGH constant.

We don't need to add a new constraint case to *arm_movsi_insn, we can just replace the current 'N' constraint case

tested on arm-none-eabi.

ok?

--
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery

2009-06-17  Nathan Sidwell  <nathan@codesourcery.com>

	* config/arm/arm.c (arm_print_operand): Deal with HIGH.
	* config/arm/constraints.md (j): New constraint for movw operands.
	(N): Remove thumb2 meaning.
	* config/arm/arm.md (*arm_movw): Delete.
	(*arm_movsi_insn): Use j constraint for movw instead of N constraint.
	* config/arm/vfp.md (*arm_movsi_vfp, *thumb2_movsi_vfp): Likewise.
	* config/arm/thumb2.md (*thumb2_movsi_insn): Likewise.

Index: config/arm/thumb2.md
===================================================================
--- config/arm/thumb2.md	(revision 148383)
+++ config/arm/thumb2.md	(working copy)
@@ -225,7 +225,7 @@
 
 (define_insn "*thumb2_movsi_insn"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,r,r,rk,m")
-	(match_operand:SI 1 "general_operand"	   "rk ,I,K,N,mi,rk"))]
+	(match_operand:SI 1 "general_operand"	   "rk ,I,K,j,mi,rk"))]
   "TARGET_THUMB2 && ! TARGET_IWMMXT
    && !(TARGET_HARD_FLOAT && TARGET_VFP)
    && (   register_operand (operands[0], SImode)
Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 148383)
+++ config/arm/arm.c	(working copy)
@@ -359,6 +359,9 @@ static bool arm_allocate_stack_slots_for
 #undef TARGET_ASM_TTYPE
 #define TARGET_ASM_TTYPE arm_output_ttype
 
+#undef TARGET_CXX_TTYPE_REF_ENCODE
+#define TARGET_CXX_TTYPE_REF_ENCODE hook_cxx_ttype_ref_in_bit0
+
 #undef TARGET_ARM_EABI_UNWINDER
 #define TARGET_ARM_EABI_UNWINDER true
 #endif /* TARGET_UNWIND_INFO */
@@ -13957,6 +13960,12 @@ arm_print_operand (FILE *stream, rtx x, 
 	default:
 	  gcc_assert (GET_CODE (x) != NEG);
 	  fputc ('#', stream);
+	  if (GET_CODE (x) == HIGH)
+	    {
+	      fputs (":lower16:", stream);
+	      x = XEXP (x, 0);
+	    }
+	    
 	  output_addr_const (stream, x);
 	  break;
 	}
@@ -19428,11 +19437,37 @@ arm_unwind_emit (FILE * asm_out_file, rt
 static bool
 arm_output_ttype (rtx x)
 {
+  rtx sym = NULL_RTX, off = NULL_RTX;
+
+  switch (GET_CODE (x))
+    {
+    case SYMBOL_REF:
+      sym = x;
+      break;
+    case CONST_INT:
+      off = x;
+      break;
+    case PLUS:
+      sym = XEXP (x, 0);
+      off = XEXP (x, 1);
+      gcc_assert (GET_CODE (sym) == SYMBOL_REF && GET_CODE (off) == CONST_INT);
+      break;
+    default:
+      gcc_unreachable ();
+    }
+  
   fputs ("\t.word\t", asm_out_file);
-  output_addr_const (asm_out_file, x);
-  /* Use special relocations for symbol references.  */
-  if (GET_CODE (x) != CONST_INT)
-    fputs ("(TARGET2)", asm_out_file);
+  if (sym)
+    {
+      output_addr_const (asm_out_file, sym);
+      /* Use special relocations for symbol references.  */
+      fputs ("(TARGET2)", asm_out_file);
+      if (off)
+	fputs (" + ", asm_out_file);
+    }
+  if (off)
+    output_addr_const (asm_out_file, sym);
+  
   fputc ('\n', asm_out_file);
 
   return TRUE;
Index: config/arm/vfp.md
===================================================================
--- config/arm/vfp.md	(revision 148383)
+++ config/arm/vfp.md	(working copy)
@@ -51,7 +51,7 @@
 ;; problems because small constants get converted into adds.
 (define_insn "*arm_movsi_vfp"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,r,r,rk,m ,*t,r,*t,*t, *Uv")
-      (match_operand:SI 1 "general_operand"	   "rk, I,K,N,mi,rk,r,*t,*t,*Uvi,*t"))]
+      (match_operand:SI 1 "general_operand"	   "rk, I,K,j,mi,rk,r,*t,*t,*Uvi,*t"))]
   "TARGET_ARM && TARGET_VFP && TARGET_HARD_FLOAT
    && (   s_register_operand (operands[0], SImode)
        || s_register_operand (operands[1], SImode))"
@@ -88,7 +88,7 @@
 
 (define_insn "*thumb2_movsi_vfp"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,r,r,rk,m,*t,r, *t,*t, *Uv")
-      (match_operand:SI 1 "general_operand"	   "rk, I,K,N,mi,rk,r,*t,*t,*Uvi,*t"))]
+      (match_operand:SI 1 "general_operand"	   "rk, I,K,j,mi,rk,r,*t,*t,*Uvi,*t"))]
   "TARGET_THUMB2 && TARGET_VFP && TARGET_HARD_FLOAT
    && (   s_register_operand (operands[0], SImode)
        || s_register_operand (operands[1], SImode))"
Index: config/arm/constraints.md
===================================================================
--- config/arm/constraints.md	(revision 148383)
+++ config/arm/constraints.md	(working copy)
@@ -25,7 +25,7 @@
 ;; In ARM state, 'l' is an alias for 'r'
 
 ;; The following normal constraints have been used:
-;; in ARM/Thumb-2 state: G, H, I, J, K, L, M
+;; in ARM/Thumb-2 state: G, H, I, j, J, K, L, M
 ;; in Thumb-1 state: I, J, K, L, M, N, O
 
 ;; The following multi-letter normal constraints have been used:
@@ -65,6 +65,13 @@
 (define_register_constraint "h" "TARGET_THUMB ? HI_REGS : NO_REGS"
  "In Thumb state the core registers @code{r8}-@code{r15}.")
 
+(define_constraint "j"
+ "A constant suitable for a MOVW instruction. (ARM/Thumb-2)"
+ (and (match_test "TARGET_32BIT && arm_arch_thumb2")
+      (ior (match_code "high")
+	   (and (match_code "const_int")
+                (match_test "(ival & 0xffff0000) == 0")))))
+
 (define_register_constraint "k" "STACK_REG"
  "@internal The stack register.")
 
@@ -116,11 +123,9 @@
 		   : ((ival >= 0 && ival <= 1020) && ((ival & 3) == 0))")))
 
 (define_constraint "N"
- "In ARM/Thumb-2 state a constant suitable for a MOVW instruction.
-  In Thumb-1 state a constant in the range 0-31."
+ "Thumb-1 state a constant in the range 0-31."
  (and (match_code "const_int")
-      (match_test "TARGET_32BIT ? arm_arch_thumb2 && ((ival & 0xffff0000) == 0)
-				: (ival >= 0 && ival <= 31)")))
+      (match_test "!TARGET_32BIT && (ival >= 0 && ival <= 31)")))
 
 (define_constraint "O"
  "In Thumb-1 state a constant that is a multiple of 4 in the range
Index: config/arm/arm.md
===================================================================
--- config/arm/arm.md	(revision 148383)
+++ config/arm/arm.md	(working copy)
@@ -4985,18 +4985,9 @@
    (set_attr "length" "4")]
 )
 
-(define_insn "*arm_movw"
-  [(set (match_operand:SI 0 "nonimmediate_operand" "=r")
-	(high:SI (match_operand:SI 1 "general_operand"      "i")))]
-  "TARGET_32BIT"
-  "movw%?\t%0, #:lower16:%c1"
-  [(set_attr "predicable" "yes")
-   (set_attr "length" "4")]
-)
-
 (define_insn "*arm_movsi_insn"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=rk,r,r,r,rk,m")
-	(match_operand:SI 1 "general_operand"      "rk, I,K,N,mi,rk"))]
+	(match_operand:SI 1 "general_operand"      "rk, I,K,j,mi,rk"))]
   "TARGET_ARM && ! TARGET_IWMMXT
    && !(TARGET_HARD_FLOAT && TARGET_VFP)
    && (   register_operand (operands[0], SImode)

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