diff mbox

Fix use of stack-pointer-register as a temporary for CRIS

Message ID 201312232234.rBNMY2X7018662@ignucius.se.axis.com
State New
Headers show

Commit Message

Hans-Peter Nilsson Dec. 23, 2013, 10:34 p.m. UTC 
The circumstances are a bit odd; the stack-pointer (sp) is never
the target for a direct assignment in "ordinary" generated code.
Still, this happens for gcc.dg/pr50251.c, calling
__builtin_stack_restore.  There's a bug in several define_splits
in the CRIS port, in that the destination of the split insn is
used as a temporary, so sp is set to something unusable as a
stack-pointer.  You don't want that in a context where
interrupts use the same stack as the running program; there's no
red-zone or anything.  Though, it *would* be valid for contexts
where the "user" stack is not the system (interrupt) stack, but
introducing that distinction is not worthwhile.

I'll mark this with middle-end/59584 only because it makes a
nice test-case should anyone want to work on the general bug
noticed there (revert the commit locally, observe ICE for
gcc.dg/pr50251.c).  The general bug for PR59584 is that GCC
can't handle fixing up the REG_ARGS_SIZE note being on a direct
assignment to the stack-pointer, therefore no define_split must
match it.  This patch just removes the define_split; the bug is
likely to hit other targets, when __builtin_stack_restore is
called.

PS. I wish we have a name field for define_splits...  I don't
think a string would collide, syntactically.  Maybe later.

Tested cris-elf, makes gcc.dg/pr50251.c pass again.

	PR middle-end/59584
	* config/cris/predicates.md (cris_nonsp_register_operand):
	New define_predicate.
	* config/cris/cris.md: Replace register_operand with
	cris_nonsp_register_operand for destinations in all
	define_splits where a register is set more than once.


brgds, H-P
diff mbox

Patch

Index: gcc/config/cris/cris.md
===================================================================
--- gcc/config/cris/cris.md	(revision 206176)
+++ gcc/config/cris/cris.md	(working copy)
@@ -758,7 +758,7 @@  (define_split
 		      (match_operand:SI 1 "const_int_operand" ""))
 	     (match_operand:SI 2 "register_operand" ""))])
 	  (match_operand 3 "register_operand" ""))
-     (set (match_operand:SI 4 "register_operand" "")
+     (set (match_operand:SI 4 "cris_nonsp_register_operand" "")
 	  (plus:SI (mult:SI (match_dup 0)
 			    (match_dup 1))
 		   (match_dup 2)))])]
@@ -859,7 +859,7 @@  (define_split
 	     (match_operand:SI 0 "cris_bdap_operand" "")
 	     (match_operand:SI 1 "cris_bdap_operand" ""))])
 	  (match_operand 2 "register_operand" ""))
-     (set (match_operand:SI 3 "register_operand" "")
+     (set (match_operand:SI 3 "cris_nonsp_register_operand" "")
 	  (plus:SI (match_dup 0) (match_dup 1)))])]
   "reload_completed && reg_overlap_mentioned_p (operands[3], operands[2])"
   [(set (match_dup 4) (match_dup 2))
@@ -3960,7 +3960,7 @@  (define_expand "casesi"
 ;; up.
 
 (define_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operator
 	 4 "cris_operand_extend_operator"
 	 [(match_operand 1 "register_operand" "")
@@ -3990,7 +3990,7 @@  (define_split
 ;; Call this op-extend-split-rx=rz
 
 (define_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operator
 	 4 "cris_plus_or_bound_operator"
 	 [(match_operand 1 "register_operand" "")
@@ -4018,7 +4018,7 @@  (define_split
 ;; Call this op-extend-split-swapped
 
 (define_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operator
 	 4 "cris_plus_or_bound_operator"
 	 [(match_operator
@@ -4044,7 +4044,7 @@  (define_split
 ;; bound.  Call this op-extend-split-swapped-rx=rz.
 
 (define_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operator
 	 4 "cris_plus_or_bound_operator"
 	 [(match_operator
@@ -4075,7 +4075,7 @@  (define_split
 ;; Call this op-extend.
 
 (define_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operator
 	 3 "cris_orthogonal_operator"
 	 [(match_operand 1 "register_operand" "")
@@ -4099,7 +4099,7 @@  (define_split
 ;; Call this op-split-rx=rz
 
 (define_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operator
 	 3 "cris_commutative_orth_op"
 	 [(match_operand 2 "memory_operand" "")
@@ -4123,7 +4123,7 @@  (define_split
 ;; Call this op-split-swapped.
 
 (define_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operator
 	 3 "cris_commutative_orth_op"
 	 [(match_operand 1 "register_operand" "")
@@ -4146,7 +4146,7 @@  (define_split
 ;; Call this op-split-swapped-rx=rz.
 
 (define_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operator
 	 3 "cris_orthogonal_operator"
 	 [(match_operand 2 "memory_operand" "")
@@ -4555,10 +4555,11 @@  (define_split
 ;; We're not allowed to generate copies of registers with different mode
 ;; until after reload; copying pseudos upsets reload.  CVS as of
 ;; 2001-08-24, unwind-dw2-fde.c, _Unwind_Find_FDE ICE in
-;; cselib_invalidate_regno.
+;; cselib_invalidate_regno.  Also, don't do this for the stack-pointer,
+;; as we don't want it set temporarily to an invalid value.
 
 (define_split ; indir_to_reg_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operand 1 "indirect_operand" ""))]
   "reload_completed
    && REG_P (operands[0])
@@ -4574,7 +4575,7 @@  (define_split ; indir_to_reg_split
 ;; As the above, but MOVS and MOVU.
 
 (define_split
-  [(set (match_operand 0 "register_operand" "")
+  [(set (match_operand 0 "cris_nonsp_register_operand" "")
 	(match_operator
 	 4 "cris_extend_operator"
 	 [(match_operand 1 "indirect_operand" "")]))]
Index: gcc/config/cris/predicates.md
===================================================================
--- gcc/config/cris/predicates.md	(revision 206176)
+++ gcc/config/cris/predicates.md	(working copy)
@@ -76,6 +76,10 @@  (define_predicate "cris_simple_operand"
 	    (match_test "cris_simple_address_operand (XEXP (op, 0),
 						      Pmode)"))))
 
+(define_predicate "cris_nonsp_register_operand"
+  (and (match_operand 0 "register_operand")
+       (match_test "op != stack_pointer_rtx")))
+
 ;; The caller needs to use :SI.
 (define_predicate "cris_bdap_sign_extend_operand"
 ; Disabled until <URL:http://gcc.gnu.org/ml/gcc-patches/2005-10/msg01376.html>