RFA: Make mn10300 use an UNSPEC instead of nested (const (minus ...))s

RFA: Make mn10300 use an UNSPEC instead of nested (const (minus ...))s

[Richard Sandiford]

This patch makes the mn10300 port use UNSPECs instead of (const (minus ...)).
It's part of my ongoing quest to enforce a grammar for CONSTs.  See:

    http://gcc.gnu.org/ml/gcc-patches/2008-10/msg00339.html

for details.

The only use of (const (minus ...)) is in:

	 (const
	  (unspec [(minus:SI
		    (match_dup 1)
		    (const (minus:SI
			    (const (match_operand:SI 0 "" ""))
			    (pc))))
		  ] UNSPEC_PIC))))]

As with SH, these nested consts appear to be there to enforce a
particular bracketing, but I don't think that behaviour is documented
or guaranteed.  The patch avoids this by adding a new unspec.

Tested by comparing the before and after assembly output for
gcc.c-torture, gcc.dg and g++.dg on mn10300-elf, using the
options:

  {-O0,-O2} x {-fpic}

There were no differences.  OK to install?

Richard


gcc/
	* config/mn10300/mn10300.h (OUTPUT_ADDR_CONST_EXTRA): Handle
	UNSPEC_GOTSYM_OFFs.
	* config/mn10300/mn10300.c (legitimate_pic_operand_p): Return true
	for UNSPEC_GOTSYM_OFFs.
	* config/mn10300/mn10300.md (UNSPEC_GOTSYM_OFF): New unspec.
	(add_GOT_to_pic_reg): Use it.
	* config/mn10300/constraints.md (S): Allow UNSPEC_GOTSYM_OFF.

Index: gcc/config/mn10300/mn10300.h
===================================================================
--- gcc/config/mn10300/mn10300.h	2008-10-10 19:13:54.000000000 +0100
+++ gcc/config/mn10300/mn10300.h	2008-10-10 19:16:29.000000000 +0100
@@ -738,7 +738,7 @@ #define MN10300_GLOBAL_P(X) (! SYMBOL_RE
    constants.  Used for PIC-specific UNSPECs.  */
 #define OUTPUT_ADDR_CONST_EXTRA(STREAM, X, FAIL) \
   do									\
-    if (GET_CODE (X) == UNSPEC && XVECLEN ((X), 0) == 1)	\
+    if (GET_CODE (X) == UNSPEC)						\
       {									\
 	switch (XINT ((X), 1))						\
 	  {								\
@@ -762,6 +762,12 @@ #define OUTPUT_ADDR_CONST_EXTRA(STREAM, 
 	    output_addr_const ((STREAM), XVECEXP ((X), 0, 0));		\
 	    fputs ("@PLT", (STREAM));					\
 	    break;							\
+	  case UNSPEC_GOTSYM_OFF:					\
+	    assemble_name (STREAM, GOT_SYMBOL_NAME);			\
+	    fputs ("-(", STREAM);					\
+	    output_addr_const (STREAM, XVECEXP (X, 0, 0));		\
+	    fputs ("-.)", STREAM);					\
+	    break;							\
 	  default:							\
 	    goto FAIL;							\
 	  }								\
Index: gcc/config/mn10300/mn10300.c
===================================================================
--- gcc/config/mn10300/mn10300.c	2008-10-10 19:13:54.000000000 +0100
+++ gcc/config/mn10300/mn10300.c	2008-10-10 19:16:24.000000000 +0100
@@ -1869,7 +1869,8 @@ legitimate_pic_operand_p (rtx x)
       && (XINT (x, 1) == UNSPEC_PIC
 	  || XINT (x, 1) == UNSPEC_GOT
 	  || XINT (x, 1) == UNSPEC_GOTOFF
-	  || XINT (x, 1) == UNSPEC_PLT))
+	  || XINT (x, 1) == UNSPEC_PLT
+	  || XINT (x, 1) == UNSPEC_GOTSYM_OFF))
       return 1;
 
   fmt = GET_RTX_FORMAT (GET_CODE (x));
Index: gcc/config/mn10300/mn10300.md
===================================================================
--- gcc/config/mn10300/mn10300.md	2008-10-10 19:13:54.000000000 +0100
+++ gcc/config/mn10300/mn10300.md	2008-10-10 19:16:24.000000000 +0100
@@ -45,6 +45,7 @@ (define_constants [
   (UNSPEC_GOT		2)
   (UNSPEC_GOTOFF	3)
   (UNSPEC_PLT		4)
+  (UNSPEC_GOTSYM_OFF	5)
 ])
 
 (include "predicates.md")
@@ -2619,18 +2620,9 @@ (define_expand "add_GOT_to_pic_reg"
   [(set (reg:SI PIC_REG)
 	(plus:SI
 	 (reg:SI PIC_REG)
-	 (const
-	  (unspec [(minus:SI
-		    (match_dup 1)
-		    (const (minus:SI
-			    (const (match_operand:SI 0 "" ""))
-			    (pc))))
-		  ] UNSPEC_PIC))))]
-  ""
-  "
-{
-  operands[1] = gen_rtx_SYMBOL_REF (VOIDmode, GOT_SYMBOL_NAME);
-}")
+	 (const:SI
+	  (unspec:SI [(match_operand:SI 0 "" "")] UNSPEC_GOTSYM_OFF))))]
+  "")
 
 (define_expand "symGOT2reg"
   [(match_operand:SI 0 "" "")
Index: gcc/config/mn10300/constraints.md
===================================================================
--- gcc/config/mn10300/constraints.md	2008-10-10 19:13:54.000000000 +0100
+++ gcc/config/mn10300/constraints.md	2008-10-10 19:16:24.000000000 +0100
@@ -68,7 +68,8 @@ (define_constraint "S"
   (if_then_else (match_test "flag_pic")
 	(and (match_test "GET_CODE (op) == UNSPEC")
 	     (ior (match_test "XINT (op, 1) == UNSPEC_PLT")
-		  (match_test "XINT (op, 1) == UNSPEC_PIC")))
+		  (match_test "XINT (op, 1) == UNSPEC_PIC")
+		  (match_test "XINT (op, 1) == UNSPEC_GOTSYM_OFF")))
 	(match_test "GET_CODE (op) == SYMBOL_REF")))
 
 ;; Integer constraints

Re: RFA: Make mn10300 use an UNSPEC instead of nested (const (minus ...))s

[Jeff Law]

Richard Sandiford wrote:
> This patch makes the mn10300 port use UNSPECs instead of (const (minus ...)).
> It's part of my ongoing quest to enforce a grammar for CONSTs.  See:
>
>     http://gcc.gnu.org/ml/gcc-patches/2008-10/msg00339.html
>
> for details.
>
> The only use of (const (minus ...)) is in:
>
> 	 (const
> 	  (unspec [(minus:SI
> 		    (match_dup 1)
> 		    (const (minus:SI
> 			    (const (match_operand:SI 0 "" ""))
> 			    (pc))))
> 		  ] UNSPEC_PIC))))]
>
> As with SH, these nested consts appear to be there to enforce a
> particular bracketing, but I don't think that behaviour is documented
> or guaranteed.  The patch avoids this by adding a new unspec.
>
> Tested by comparing the before and after assembly output for
> gcc.c-torture, gcc.dg and g++.dg on mn10300-elf, using the
> options:
>
>   {-O0,-O2} x {-fpic}
>
> There were no differences.  OK to install?
>
> Richard
>
>
> gcc/
> 	* config/mn10300/mn10300.h (OUTPUT_ADDR_CONST_EXTRA): Handle
> 	UNSPEC_GOTSYM_OFFs.
> 	* config/mn10300/mn10300.c (legitimate_pic_operand_p): Return true
> 	for UNSPEC_GOTSYM_OFFs.
> 	* config/mn10300/mn10300.md (UNSPEC_GOTSYM_OFF): New unspec.
> 	(add_GOT_to_pic_reg): Use it.
> 	* config/mn10300/constraints.md (S): Allow UNSPEC_GOTSYM_OFF.
>   
This is fine.

I can only guess the idea was that we could CSE the inner subtraction, 
but I can't see how that would work either.

Jeff