diff mbox

Add new alu_type and logical_type attributes for MIPS.

Message ID 4C23D5A8.9040609@codesourcery.com
State New
Headers show

Commit Message

Catherine Moore June 24, 2010, 10:01 p.m. UTC 
Hi Richard,

This patch adds the alu_type attribute and the logical_type attribute. 
    The purpose of this patch is to lay the groundwork for the removal 
of the micromips_type attribute in the pending microMIPS patch.  Does 
this look okay to install?

Test results looked good for the mips-sde-elf target.

Thanks,
Catherine


2010-06-24  Catherine Moore  <clm@codesourcery.com>

         * config/mips/mips.md (alu_type): New attribute.
         (logical_type): New attribute.
         (type): Infer type from alu_type and logical_type.
         (*add<mode>3, *add<mode>3_mips16, *addsi3_extended,
         *baddu_si_eb, *baddu_si_el, *baddu_di, sub<mode>3,
         *subsi3_extended, negsi2, negdi2, *low<mode>,
         *low<mode>_mips16): Set alu_type instead
         of type.
         (*ior<mode>3, *ior<mode>3_mips16, xor<mode>3, *nor<mode>3,
         *zero_extend<GPR:mode>_trunc<SHORT:mode>,
         *zero_extendhi_truncqi: Set logical_type instead of type.

Comments

Richard Sandiford June 26, 2010, 5:38 p.m. UTC | #1 
Catherine Moore <clm@codesourcery.com> writes:
> 2010-06-24  Catherine Moore  <clm@codesourcery.com>
>
>          * config/mips/mips.md (alu_type): New attribute.
>          (logical_type): New attribute.
>          (type): Infer type from alu_type and logical_type.
>          (*add<mode>3, *add<mode>3_mips16, *addsi3_extended,
>          *baddu_si_eb, *baddu_si_el, *baddu_di, sub<mode>3,
>          *subsi3_extended, negsi2, negdi2, *low<mode>,
>          *low<mode>_mips16): Set alu_type instead
>          of type.
>          (*ior<mode>3, *ior<mode>3_mips16, xor<mode>3, *nor<mode>3,
>          *zero_extend<GPR:mode>_trunc<SHORT:mode>,
>          *zero_extendhi_truncqi: Set logical_type instead of type.

Thanks, looks good.

> +(define_attr "alu_type" "unknown,add,sub"
> +  (const_string "unknown"))
> +
> +(define_attr "logical_type" "unknown,not,and,or,xor"
> +  (const_string "unknown"))

Sorry to be a pain, but I'd imagined "not", "and", "or" and "xor" being
part of alu_type.  It's conceivable that some alternatives for a pattern
might use arithmetic ops while others use logical ones.

Also, I think we should either add "nor" or fold "xor" into "or";
it seems inconsistent to have one and not the other.

> +	 (eq_attr "alu_type" "add") (const_string "arith")
> +	 (eq_attr "alu_type" "sub") (const_string "arith")

Can be simplified to:

  (eq_attr "alu_type" "add,sub") (const_string "arith")

> +	 (eq_attr "logical_type" "not") (const_string "logical")
> +	 (eq_attr "logical_type" "and") (const_string "logical")
> +	 (eq_attr "logical_type" "or")  (const_string "logical")
> +	 (eq_attr "logical_type" "xor") (const_string "logical")

Same idea here.

> @@ -2665,7 +2679,7 @@ (define_insn ""
>    "@
>     xor\t%0,%1,%2
>     xori\t%0,%1,%x2"
> -  [(set_attr "type" "logical")
> +  [(set_attr "logical_type" "or")
>     (set_attr "mode" "<MODE>")])

s/or/xor/ if we keep the xor distinction.

> @@ -2678,6 +2692,7 @@ (define_insn ""
>     cmpi\t%1,%2
>     cmp\t%1,%2"
>    [(set_attr "type" "logical,arith,arith")
> +   (set_attr "logical_type" "or,unknown,unknown")
>     (set_attr "mode" "<MODE>")
>     (set_attr_alternative "length"
>  		[(const_int 4)

Actually, the existing type attribute is wrong here.  CMP is just a
special (and somewhat confusingly-named) type of XOR.  So you should
just be able to remove the type attribute and use:

  (set_attr "alu_type" "xor")

(No need for it to be "xor,xor,xor".)  s/xor/or/ if we drop the
or,xor,... distinction.

> @@ -2692,7 +2707,7 @@ (define_insn "*nor<mode>3"
>  		 (not:GPR (match_operand:GPR 2 "register_operand" "d"))))]
>    "!TARGET_MIPS16"
>    "nor\t%0,%1,%2"
> -  [(set_attr "type" "logical")
> +  [(set_attr "logical_type" "or")
>     (set_attr "mode" "<MODE>")])

s/or/nor/ if we keep the or,xor,... distinction.

FTR, I realise this doesn't update mips-dsp*.md or mips-fixed.md,
but I think that's a good thing.

OK with those changes, thanks.

Richard
Catherine Moore June 26, 2010, 7:28 p.m. UTC | #2 
Richard Sandiford wrote:

> FTR, I realise this doesn't update mips-dsp*.md or mips-fixed.md,
> but I think that's a good thing.
> 
   I thought it was a good thing too.

> OK with those changes, thanks.
> 

Thanks for the comments.  Patch committed with your suggested changes.

Catherine
diff mbox

Patch

Index: mips.md
===================================================================
--- mips.md	(revision 161336)
+++ mips.md	(working copy)
@@ -199,6 +199,12 @@  (define_attr "move_type"
    shift_shift,lui_movf"
   (const_string "unknown"))
 
+(define_attr "alu_type" "unknown,add,sub"
+  (const_string "unknown"))
+
+(define_attr "logical_type" "unknown,not,and,or,xor"
+  (const_string "unknown"))
+
 ;; Main data type used by the insn
 (define_attr "mode" "unknown,none,QI,HI,SI,DI,TI,SF,DF,TF,FPSW"
   (const_string "unknown"))
@@ -275,6 +281,14 @@  (define_attr "type"
   (cond [(eq_attr "jal" "!unset") (const_string "call")
 	 (eq_attr "got" "load") (const_string "load")
 
+	 (eq_attr "alu_type" "add") (const_string "arith")
+	 (eq_attr "alu_type" "sub") (const_string "arith")
+
+	 (eq_attr "logical_type" "not") (const_string "logical")
+	 (eq_attr "logical_type" "and") (const_string "logical")
+	 (eq_attr "logical_type" "or")  (const_string "logical")
+	 (eq_attr "logical_type" "xor") (const_string "logical")
+
 	 ;; If a doubleword move uses these expensive instructions,
 	 ;; it is usually better to schedule them in the same way
 	 ;; as the singleword form, rather than as "multi".
@@ -978,7 +992,7 @@  (define_insn "*add<mode>3"
   "@
     <d>addu\t%0,%1,%2
     <d>addiu\t%0,%1,%2"
-  [(set_attr "type" "arith")
+  [(set_attr "alu_type" "add")
    (set_attr "mode" "<MODE>")])
 
 (define_insn "*add<mode>3_mips16"
@@ -992,7 +1006,7 @@  (define_insn "*add<mode>3_mips16"
     <d>addiu\t%0,%2
     <d>addiu\t%0,%1,%2
     <d>addu\t%0,%1,%2"
-  [(set_attr "type" "arith")
+  [(set_attr "alu_type" "add")
    (set_attr "mode" "<MODE>")
    (set_attr_alternative "length"
 		[(if_then_else (match_operand 2 "m16_simm8_8")
@@ -1130,7 +1144,7 @@  (define_insn "*addsi3_extended"
   "@
     addu\t%0,%1,%2
     addiu\t%0,%1,%2"
-  [(set_attr "type" "arith")
+  [(set_attr "alu_type" "add")
    (set_attr "mode" "SI")])
 
 ;; Split this insn so that the addiu splitters can have a crack at it.
@@ -1145,7 +1159,7 @@  (define_insn_and_split "*addsi3_extended
   "&& reload_completed"
   [(set (match_dup 3) (plus:SI (match_dup 1) (match_dup 2)))]
   { operands[3] = gen_lowpart (SImode, operands[0]); }
-  [(set_attr "type" "arith")
+  [(set_attr "alu_type" "add")
    (set_attr "mode" "SI")
    (set_attr "extended_mips16" "yes")])
 
@@ -1159,7 +1173,7 @@  (define_insn "*baddu_si_eb"
 		   (match_operand:SI 2 "register_operand" "d")) 3)))]
   "ISA_HAS_BADDU && BYTES_BIG_ENDIAN"
   "baddu\\t%0,%1,%2"
-  [(set_attr "type" "arith")])
+  [(set_attr "alu_type" "add")])
 
 (define_insn "*baddu_si_el"
   [(set (match_operand:SI 0 "register_operand" "=d")
@@ -1169,7 +1183,7 @@  (define_insn "*baddu_si_el"
 		   (match_operand:SI 2 "register_operand" "d")) 0)))]
   "ISA_HAS_BADDU && !BYTES_BIG_ENDIAN"
   "baddu\\t%0,%1,%2"
-  [(set_attr "type" "arith")])
+  [(set_attr "alu_type" "add")])
 
 (define_insn "*baddu_di<mode>"
   [(set (match_operand:GPR 0 "register_operand" "=d")
@@ -1179,7 +1193,7 @@  (define_insn "*baddu_di<mode>"
 		   (match_operand:DI 2 "register_operand" "d")))))]
   "ISA_HAS_BADDU && TARGET_64BIT"
   "baddu\\t%0,%1,%2"
-  [(set_attr "type" "arith")])
+  [(set_attr "alu_type" "add")])
 
 ;;
 ;;  ....................
@@ -1204,7 +1218,7 @@  (define_insn "sub<mode>3"
 		   (match_operand:GPR 2 "register_operand" "d")))]
   ""
   "<d>subu\t%0,%1,%2"
-  [(set_attr "type" "arith")
+  [(set_attr "alu_type" "sub")
    (set_attr "mode" "<MODE>")])
 
 (define_insn "*subsi3_extended"
@@ -1214,7 +1228,7 @@  (define_insn "*subsi3_extended"
 		      (match_operand:SI 2 "register_operand" "d"))))]
   "TARGET_64BIT"
   "subu\t%0,%1,%2"
-  [(set_attr "type" "arith")
+  [(set_attr "alu_type" "sub")
    (set_attr "mode" "DI")])
 
 ;;
@@ -2483,7 +2497,7 @@  (define_insn "negsi2"
   else
     return "subu\t%0,%.,%1";
 }
-  [(set_attr "type"	"arith")
+  [(set_attr "alu_type"	"sub")
    (set_attr "mode"	"SI")])
 
 (define_insn "negdi2"
@@ -2491,7 +2505,7 @@  (define_insn "negdi2"
 	(neg:DI (match_operand:DI 1 "register_operand" "d")))]
   "TARGET_64BIT && !TARGET_MIPS16"
   "dsubu\t%0,%.,%1"
-  [(set_attr "type"	"arith")
+  [(set_attr "alu_type"	"sub")
    (set_attr "mode"	"DI")])
 
 ;; neg.fmt is an arithmetic instruction and treats all NaN inputs as
@@ -2516,7 +2530,7 @@  (define_insn "one_cmpl<mode>2"
   else
     return "nor\t%0,%.,%1";
 }
-  [(set_attr "type" "logical")
+  [(set_attr "logical_type" "not")
    (set_attr "mode" "<MODE>")])
 
 ;;
@@ -2638,7 +2652,7 @@  (define_insn "*ior<mode>3"
   "@
    or\t%0,%1,%2
    ori\t%0,%1,%x2"
-  [(set_attr "type" "logical")
+  [(set_attr "logical_type" "or")
    (set_attr "mode" "<MODE>")])
 
 (define_insn "*ior<mode>3_mips16"
@@ -2647,7 +2661,7 @@  (define_insn "*ior<mode>3_mips16"
 		 (match_operand:GPR 2 "register_operand" "d")))]
   "TARGET_MIPS16"
   "or\t%0,%2"
-  [(set_attr "type" "logical")
+  [(set_attr "logical_type" "or")
    (set_attr "mode" "<MODE>")])
 
 (define_expand "xor<mode>3"
@@ -2665,7 +2679,7 @@  (define_insn ""
   "@
    xor\t%0,%1,%2
    xori\t%0,%1,%x2"
-  [(set_attr "type" "logical")
+  [(set_attr "logical_type" "or")
    (set_attr "mode" "<MODE>")])
 
 (define_insn ""
@@ -2678,6 +2692,7 @@  (define_insn ""
    cmpi\t%1,%2
    cmp\t%1,%2"
   [(set_attr "type" "logical,arith,arith")
+   (set_attr "logical_type" "or,unknown,unknown")
    (set_attr "mode" "<MODE>")
    (set_attr_alternative "length"
 		[(const_int 4)
@@ -2692,7 +2707,7 @@  (define_insn "*nor<mode>3"
 		 (not:GPR (match_operand:GPR 2 "register_operand" "d"))))]
   "!TARGET_MIPS16"
   "nor\t%0,%1,%2"
-  [(set_attr "type" "logical")
+  [(set_attr "logical_type" "or")
    (set_attr "mode" "<MODE>")])
 
 ;;
@@ -2910,7 +2925,7 @@  (define_insn "*zero_extend<GPR:mode>_tru
   operands[2] = GEN_INT (GET_MODE_MASK (<SHORT:MODE>mode));
   return "andi\t%0,%1,%x2";
 }
-  [(set_attr "type" "logical")
+  [(set_attr "logical_type" "and")
    (set_attr "mode" "<GPR:MODE>")])
 
 (define_insn "*zero_extendhi_truncqi"
@@ -2919,7 +2934,7 @@  (define_insn "*zero_extendhi_truncqi"
 	    (truncate:QI (match_operand:DI 1 "register_operand" "d"))))]
   "TARGET_64BIT && !TARGET_MIPS16"
   "andi\t%0,%1,0xff"
-  [(set_attr "type" "logical")
+  [(set_attr "logical_type" "and")
    (set_attr "mode" "HI")])
 
 ;;
@@ -3851,7 +3866,7 @@  (define_insn "*low<mode>"
 		  (match_operand:P 2 "immediate_operand" "")))]
   "!TARGET_MIPS16"
   "<d>addiu\t%0,%1,%R2"
-  [(set_attr "type" "arith")
+  [(set_attr "alu_type" "add")
    (set_attr "mode" "<MODE>")])
 
 (define_insn "*low<mode>_mips16"
@@ -3860,7 +3875,7 @@  (define_insn "*low<mode>_mips16"
 		  (match_operand:P 2 "immediate_operand" "")))]
   "TARGET_MIPS16"
   "<d>addiu\t%0,%R2"
-  [(set_attr "type" "arith")
+  [(set_attr "alu_type" "add")
    (set_attr "mode" "<MODE>")
    (set_attr "extended_mips16" "yes")])