public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Add MIPS64r2 support to <u>mulsidi3_64bit
@ 2009-10-23 19:30 Adam Nemet
  2009-10-24 10:06 ` Richard Sandiford
  0 siblings, 1 reply; 6+ messages in thread
From: Adam Nemet @ 2009-10-23 19:30 UTC (permalink / raw)
  To: gcc-patches

Richard writes in <http://gcc.gnu.org/ml/gcc-patches/2009-10/msg00775.html>:
> Adam Nemet <anemet@caviumnetworks.com> writes:
> > (OTOH I was also working on a 64r2 patch that replaces the four insns
> > that combine the two halves with one bit-insertion.  So we will be
> > down to one additional insn on 64r2.)
> 
> Sounds good.  And probably suitable for stage 3 too.

Here it is.

To summarize, the patch optimizes the sequence we generate for
<u>mulsidi3_64bit with ISA_HAS_EXT_INS.  The basic sequence is:

  (1) mult<u> op1, op2
  (2) mflo t
  (3) mfhi op0
  (4) dsll t, 32
  (5) dsrl t, 32
  (6) dsll op0, 32
  (7) or   op0, t

The new sequence with DINS is:

  (1) mult<u> op1, op2
  (2) mflo op0
  (3) mfhi t
  (4) dins op0, t, 32, 32

As you can see op0 and t have to be swapped for the DINS sequence.  Since we
can I think it's better to have them the same way in the basic sequence for
consistency:

  (1) mult<u> op1, op2
  (2) mflo op0
  (3) mfhi t
  (4) dsll op0, 32
  (5) dsrl op0, 32
  (6) dsll t, 32
  (7) or   op0, t

I've considered merging the common parts of the resulting two splitters.  One
idea I had was to always split into a bit-insertion which would be further
split if DINS was unavailable.  It got messy though because for the basic
sequence we clobber 't', so the bit-insertion would have to be coupled with a
clobber.  I.e. the bit-insertion lowering splitter would look like this:

(define_split
  [(set (zero_extract:DI (match_operand:DI 0 "register_operand")
			 (const_int 32)
			 (const_int 32))
	(match_operand:DI 1 "register_operand"))
   (clobber (match_operand:DI 2 "register_operand"))]
   "TARGET_64BIT && !ISA_HAS_EXT_INS && reload_completed"
   [;; Zero extend OP0 in place.
    (set (match_dup 0) (ashift:DI (match_dup 0) (const_int 32)))
    (set (match_dup 0) (lshiftrt:DI (match_dup 0) (const_int 32)))
    ;; Shift OP1 in place.
    (set (match_dup 2) (ashift:DI (match_dup 1) (const_int 32)))
    ;; OR the two halves together.
    (set (match_dup 0) (ior:DI (match_dup 0) (match_dup 2)))])

with the scratch being a duplicate of op1 in the caller.  Unfortunately now
you would have to have a splitter (or define_insn) to remove the clobber for
bit-insertion as well.  You can get around this by emitting from C but that's
not worth it IMO.  So I just kept the two independent splitters.

Boostrapped and regtested on mips64octeon-linux-gnu.  Regtested on
mipsisa64-elf and mipsisa64r2-elf.

OK for mainline?

Adam


	* config/mips/predicates.md (hilo_operand): New predicate.
	* config/mips/mips.md (<u>mulsidi3_64bit): Change it to a
	define_insn.  Correct !ISA_HAS_EXT_INS length from 24 to 28.  Move
	splitter part from here ...:
	(<u>mulsidi3_64bit splitter for !ISA_HAS_EXT_INS): ... to here.  Swap
	op0 and op4 to match the DINS case.
	(<u>mulsidi3_64bit splitter for ISA_HAS_EXT_INS): New splitter.

testsuite/
	* gcc.target/mips/mult-1.c: New test.

Index: gcc/config/mips/predicates.md
===================================================================
--- gcc.orig/config/mips/predicates.md	2009-10-22 11:07:38.000000000 -0700
+++ gcc/config/mips/predicates.md	2009-10-22 11:16:16.000000000 -0700
@@ -119,6 +119,10 @@ (define_predicate "lo_operand"
   (and (match_code "reg")
        (match_test "REGNO (op) == LO_REGNUM")))
 
+(define_predicate "hilo_operand"
+  (and (match_code "reg")
+       (match_test "MD_REG_P (REGNO (op))")))
+
 (define_predicate "fcc_reload_operand"
   (and (match_code "reg,subreg")
        (match_test "ST_REG_P (true_regnum (op))")))
Index: gcc/config/mips/mips.md
===================================================================
--- gcc.orig/config/mips/mips.md	2009-10-22 10:39:26.000000000 -0700
+++ gcc/config/mips/mips.md	2009-10-22 12:17:22.000000000 -0700
@@ -1879,7 +1879,7 @@ (define_insn "<u>mulsidi3_32bit_r4000"
    (set_attr "mode" "SI")
    (set_attr "length" "12")])
 
-(define_insn_and_split "<u>mulsidi3_64bit"
+(define_insn "<u>mulsidi3_64bit"
   [(set (match_operand:DI 0 "register_operand" "=d")
 	(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand" "d"))
 		 (any_extend:DI (match_operand:SI 2 "register_operand" "d"))))
@@ -1887,37 +1887,67 @@ (define_insn_and_split "<u>mulsidi3_64bi
    (clobber (match_scratch:DI 4 "=d"))]
   "TARGET_64BIT && !TARGET_FIX_R4000"
   "#"
-  "&& reload_completed"
+  [(set_attr "type" "imul")
+   (set_attr "mode" "SI")
+   (set (attr "length")
+	(if_then_else (ne (symbol_ref "ISA_HAS_EXT_INS") (const_int 0))
+		      (const_int 16)
+		      (const_int 28)))])
+
+(define_split
+  [(set (match_operand:DI 0 "register_operand")
+	(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand"))
+		 (any_extend:DI (match_operand:SI 2 "register_operand"))))
+   (clobber (match_operand:TI 3 "hilo_operand"))
+   (clobber (match_operand:DI 4 "register_operand"))]
+  "TARGET_64BIT && ISA_HAS_EXT_INS && !TARGET_FIX_R4000 && reload_completed"
   [(set (match_dup 3)
 	(unspec:TI [(mult:DI (any_extend:DI (match_dup 1))
 			     (any_extend:DI (match_dup 2)))]
 		   UNSPEC_SET_HILO))
 
-   ;; OP4 <- LO, OP0 <- HI
-   (set (match_dup 4) (match_dup 5))
-   (set (match_dup 0) (unspec:DI [(match_dup 3)] UNSPEC_MFHI))
+   ;; OP0 <- LO, OP4 <- HI
+   (set (match_dup 0) (match_dup 5))
+   (set (match_dup 4) (unspec:DI [(match_dup 3)] UNSPEC_MFHI))
+
+   (set (zero_extract:DI (match_dup 0) (const_int 32) (const_int 32))
+	(match_dup 4))]
+  { operands[5] = gen_rtx_REG (DImode, LO_REGNUM); })
+
+(define_split
+  [(set (match_operand:DI 0 "register_operand")
+	(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand"))
+		 (any_extend:DI (match_operand:SI 2 "register_operand"))))
+   (clobber (match_operand:TI 3 "hilo_operand"))
+   (clobber (match_operand:DI 4 "register_operand"))]
+  "TARGET_64BIT && !ISA_HAS_EXT_INS && !TARGET_FIX_R4000 && reload_completed"
+  [(set (match_dup 3)
+	(unspec:TI [(mult:DI (any_extend:DI (match_dup 1))
+			     (any_extend:DI (match_dup 2)))]
+		   UNSPEC_SET_HILO))
+
+   ;; OP0 <- LO, OP4 <- HI
+   (set (match_dup 0) (match_dup 5))
+   (set (match_dup 4) (unspec:DI [(match_dup 3)] UNSPEC_MFHI))
 
    ;; Zero-extend OP4.
-   (set (match_dup 4)
-	(ashift:DI (match_dup 4)
+   (set (match_dup 0)
+	(ashift:DI (match_dup 0)
 		   (const_int 32)))
-   (set (match_dup 4)
-	(lshiftrt:DI (match_dup 4)
+   (set (match_dup 0)
+	(lshiftrt:DI (match_dup 0)
 		     (const_int 32)))
 
    ;; Shift OP0 into place.
-   (set (match_dup 0)
-	(ashift:DI (match_dup 0)
+   (set (match_dup 4)
+	(ashift:DI (match_dup 4)
 		   (const_int 32)))
 
    ;; OR the two halves together
    (set (match_dup 0)
 	(ior:DI (match_dup 0)
 		(match_dup 4)))]
-  { operands[5] = gen_rtx_REG (DImode, LO_REGNUM); }
-  [(set_attr "type" "imul")
-   (set_attr "mode" "SI")
-   (set_attr "length" "24")])
+  { operands[5] = gen_rtx_REG (DImode, LO_REGNUM); })
 
 (define_insn "<u>mulsidi3_64bit_hilo"
   [(set (match_operand:TI 0 "register_operand" "=x")
Index: gcc/testsuite/gcc.target/mips/mult-1.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.target/mips/mult-1.c	2009-10-22 10:41:11.000000000 -0700
@@ -0,0 +1,14 @@
+/* For SI->DI widening multiplication we should use DINS to combine the two
+   halves.  */
+/* { dg-options "-O -mgp64 isa_rev>=2" } */
+/* { dg-final { scan-assembler "\tdins\t" } } */
+/* { dg-final { scan-assembler-not "\tdsll\t" } } */
+/* { dg-final { scan-assembler-not "\tdsrl\t" } } */
+/* { dg-final { scan-assembler-not "\tor\t" } } */
+
+NOMIPS16 unsigned long long
+f (unsigned int i, unsigned int j)
+{
+  i++;
+  return (unsigned long long) i * j;
+}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add MIPS64r2 support to <u>mulsidi3_64bit
  2009-10-23 19:30 [PATCH] Add MIPS64r2 support to <u>mulsidi3_64bit Adam Nemet
@ 2009-10-24 10:06 ` Richard Sandiford
  2009-10-24 15:54   ` Adam Nemet
  2009-10-25 10:49   ` Adam Nemet
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Sandiford @ 2009-10-24 10:06 UTC (permalink / raw)
  To: Adam Nemet; +Cc: gcc-patches

Adam Nemet <anemet@caviumnetworks.com> writes:
> Richard writes in <http://gcc.gnu.org/ml/gcc-patches/2009-10/msg00775.html>:
>> Adam Nemet <anemet@caviumnetworks.com> writes:
>> > (OTOH I was also working on a 64r2 patch that replaces the four insns
>> > that combine the two halves with one bit-insertion.  So we will be
>> > down to one additional insn on 64r2.)
>> 
>> Sounds good.  And probably suitable for stage 3 too.
>
> Here it is.
>
> To summarize, the patch optimizes the sequence we generate for
> <u>mulsidi3_64bit with ISA_HAS_EXT_INS.  The basic sequence is:
>
>   (1) mult<u> op1, op2
>   (2) mflo t
>   (3) mfhi op0
>   (4) dsll t, 32
>   (5) dsrl t, 32
>   (6) dsll op0, 32
>   (7) or   op0, t
>
> The new sequence with DINS is:
>
>   (1) mult<u> op1, op2
>   (2) mflo op0
>   (3) mfhi t
>   (4) dins op0, t, 32, 32
>
> As you can see op0 and t have to be swapped for the DINS sequence.  Since we
> can I think it's better to have them the same way in the basic sequence for
> consistency:
>
>   (1) mult<u> op1, op2
>   (2) mflo op0
>   (3) mfhi t
>   (4) dsll op0, 32
>   (5) dsrl op0, 32
>   (6) dsll t, 32
>   (7) or   op0, t

Agreed.

> I've considered merging the common parts of the resulting two splitters.  One
> idea I had was to always split into a bit-insertion which would be further
> split if DINS was unavailable.  It got messy though because for the basic
> sequence we clobber 't', so the bit-insertion would have to be coupled with a
> clobber.  I.e. the bit-insertion lowering splitter would look like this:
>
> (define_split
>   [(set (zero_extract:DI (match_operand:DI 0 "register_operand")
> 			 (const_int 32)
> 			 (const_int 32))
> 	(match_operand:DI 1 "register_operand"))
>    (clobber (match_operand:DI 2 "register_operand"))]
>    "TARGET_64BIT && !ISA_HAS_EXT_INS && reload_completed"
>    [;; Zero extend OP0 in place.
>     (set (match_dup 0) (ashift:DI (match_dup 0) (const_int 32)))
>     (set (match_dup 0) (lshiftrt:DI (match_dup 0) (const_int 32)))
>     ;; Shift OP1 in place.
>     (set (match_dup 2) (ashift:DI (match_dup 1) (const_int 32)))
>     ;; OR the two halves together.
>     (set (match_dup 0) (ior:DI (match_dup 0) (match_dup 2)))])
>
> with the scratch being a duplicate of op1 in the caller.  Unfortunately now
> you would have to have a splitter (or define_insn) to remove the clobber for
> bit-insertion as well.  You can get around this by emitting from C but that's
> not worth it IMO.  So I just kept the two independent splitters.

Agreed here too.

> +(define_split
> +  [(set (match_operand:DI 0 "register_operand")
> +	(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand"))
> +		 (any_extend:DI (match_operand:SI 2 "register_operand"))))
> +   (clobber (match_operand:TI 3 "hilo_operand"))
> +   (clobber (match_operand:DI 4 "register_operand"))]
> +  "TARGET_64BIT && ISA_HAS_EXT_INS && !TARGET_FIX_R4000 && reload_completed"

Very minor, but I find it more readable when the first part of the condition
exactly matches the associated define_insn, i.e.:

  "TARGET_64BIT && !TARGET_FIX_R4000 && ISA_HAS_EXT_INS && reload_completed"

It seems a bit inconsistent to add hilo_operand while continuing to use
register_operand rather than d_operand for the rest.  I'd prefer to use
register_operand for everything or specific operands for everything.
I suppose specific operands are better, so lets go with d_operand.
And sorry in advance if there are other examples of this.

Same with the other split.

OK for trunk with those changes, thanks.

Oh, and good catch on the faulty length.  Oops!

Richard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add MIPS64r2 support to <u>mulsidi3_64bit
  2009-10-24 10:06 ` Richard Sandiford
@ 2009-10-24 15:54   ` Adam Nemet
  2009-10-25 10:49   ` Adam Nemet
  1 sibling, 0 replies; 6+ messages in thread
From: Adam Nemet @ 2009-10-24 15:54 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

Richard Sandiford writes:
> It seems a bit inconsistent to add hilo_operand while continuing to use
> register_operand rather than d_operand for the rest.  I'd prefer to use
> register_operand for everything or specific operands for everything.
> I suppose specific operands are better, so lets go with d_operand.

Absolutely, I wondered about that too but couldn't reach a conclusion about
existing practice -- thanks for clarifying.

Adam

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add MIPS64r2 support to <u>mulsidi3_64bit
  2009-10-24 10:06 ` Richard Sandiford
  2009-10-24 15:54   ` Adam Nemet
@ 2009-10-25 10:49   ` Adam Nemet
  2009-11-03 21:29     ` Richard Sandiford
  1 sibling, 1 reply; 6+ messages in thread
From: Adam Nemet @ 2009-10-25 10:49 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches, hutchinsonandy

Richard Sandiford writes:
> Very minor, but I find it more readable when the first part of the condition
> exactly matches the associated define_insn, i.e.:
> 
>   "TARGET_64BIT && !TARGET_FIX_R4000 && ISA_HAS_EXT_INS && reload_completed"
> 
> It seems a bit inconsistent to add hilo_operand while continuing to use
> register_operand rather than d_operand for the rest.  I'd prefer to use
> register_operand for everything or specific operands for everything.
> I suppose specific operands are better, so lets go with d_operand.
> And sorry in advance if there are other examples of this.
> 
> Same with the other split.
> 
> OK for trunk with those changes, thanks.

This is what I've checked in after retesting as before:

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 153537)
+++ ChangeLog	(working copy)
@@ -1,4 +1,14 @@
-	2009-09-27  Andy Hutchinson  <hutchinsonandy@gcc.gnu.org>
+2009-10-24  Adam Nemet  <anemet@caviumnetworks.com>
+
+	* config/mips/predicates.md (hilo_operand): New predicate.
+	* config/mips/mips.md (<u>mulsidi3_64bit): Change it to a
+	define_insn.  Correct !ISA_HAS_EXT_INS length from 24 to 28.  Move
+	splitter part from here ...:
+	(<u>mulsidi3_64bit splitter for !ISA_HAS_EXT_INS): ... to here.  Swap
+	op0 and op4 to match the DINS case.
+	(<u>mulsidi3_64bit splitter for ISA_HAS_EXT_INS): New splitter.
+
+2009-10-24  Andy Hutchinson  <hutchinsonandy@gcc.gnu.org>
 
 	PR middle-end/19154
 	* avr.md (QIDI): Add new mode iterator.
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog	(revision 153537)
+++ testsuite/ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2009-10-24  Adam Nemet  <anemet@caviumnetworks.com>
+
+	* gcc.target/mips/mult-1.c: New test.
+
 2009-10-24  Janus Weil  <janus@gcc.gnu.org>
 
 	PR fortran/41784
Index: config/mips/predicates.md
===================================================================
--- config/mips/predicates.md	(revision 152466)
+++ config/mips/predicates.md	(working copy)
@@ -119,6 +119,10 @@ (define_predicate "lo_operand"
   (and (match_code "reg")
        (match_test "REGNO (op) == LO_REGNUM")))
 
+(define_predicate "hilo_operand"
+  (and (match_code "reg")
+       (match_test "MD_REG_P (REGNO (op))")))
+
 (define_predicate "fcc_reload_operand"
   (and (match_code "reg,subreg")
        (match_test "ST_REG_P (true_regnum (op))")))
Index: config/mips/mips.md
===================================================================
--- config/mips/mips.md	(revision 152466)
+++ config/mips/mips.md	(working copy)
@@ -1879,7 +1879,7 @@ (define_insn "<u>mulsidi3_32bit_r4000"
    (set_attr "mode" "SI")
    (set_attr "length" "12")])
 
-(define_insn_and_split "<u>mulsidi3_64bit"
+(define_insn "<u>mulsidi3_64bit"
   [(set (match_operand:DI 0 "register_operand" "=d")
 	(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand" "d"))
 		 (any_extend:DI (match_operand:SI 2 "register_operand" "d"))))
@@ -1887,37 +1887,67 @@ (define_insn_and_split "<u>mulsidi3_64bi
    (clobber (match_scratch:DI 4 "=d"))]
   "TARGET_64BIT && !TARGET_FIX_R4000"
   "#"
-  "&& reload_completed"
+  [(set_attr "type" "imul")
+   (set_attr "mode" "SI")
+   (set (attr "length")
+	(if_then_else (ne (symbol_ref "ISA_HAS_EXT_INS") (const_int 0))
+		      (const_int 16)
+		      (const_int 28)))])
+
+(define_split
+  [(set (match_operand:DI 0 "d_operand")
+	(mult:DI (any_extend:DI (match_operand:SI 1 "d_operand"))
+		 (any_extend:DI (match_operand:SI 2 "d_operand"))))
+   (clobber (match_operand:TI 3 "hilo_operand"))
+   (clobber (match_operand:DI 4 "d_operand"))]
+  "TARGET_64BIT && !TARGET_FIX_R4000 && ISA_HAS_EXT_INS && reload_completed"
   [(set (match_dup 3)
 	(unspec:TI [(mult:DI (any_extend:DI (match_dup 1))
 			     (any_extend:DI (match_dup 2)))]
 		   UNSPEC_SET_HILO))
 
-   ;; OP4 <- LO, OP0 <- HI
-   (set (match_dup 4) (match_dup 5))
-   (set (match_dup 0) (unspec:DI [(match_dup 3)] UNSPEC_MFHI))
+   ;; OP0 <- LO, OP4 <- HI
+   (set (match_dup 0) (match_dup 5))
+   (set (match_dup 4) (unspec:DI [(match_dup 3)] UNSPEC_MFHI))
+
+   (set (zero_extract:DI (match_dup 0) (const_int 32) (const_int 32))
+	(match_dup 4))]
+  { operands[5] = gen_rtx_REG (DImode, LO_REGNUM); })
+
+(define_split
+  [(set (match_operand:DI 0 "d_operand")
+	(mult:DI (any_extend:DI (match_operand:SI 1 "d_operand"))
+		 (any_extend:DI (match_operand:SI 2 "d_operand"))))
+   (clobber (match_operand:TI 3 "hilo_operand"))
+   (clobber (match_operand:DI 4 "d_operand"))]
+  "TARGET_64BIT && !TARGET_FIX_R4000 && !ISA_HAS_EXT_INS && reload_completed"
+  [(set (match_dup 3)
+	(unspec:TI [(mult:DI (any_extend:DI (match_dup 1))
+			     (any_extend:DI (match_dup 2)))]
+		   UNSPEC_SET_HILO))
+
+   ;; OP0 <- LO, OP4 <- HI
+   (set (match_dup 0) (match_dup 5))
+   (set (match_dup 4) (unspec:DI [(match_dup 3)] UNSPEC_MFHI))
 
    ;; Zero-extend OP4.
-   (set (match_dup 4)
-	(ashift:DI (match_dup 4)
+   (set (match_dup 0)
+	(ashift:DI (match_dup 0)
 		   (const_int 32)))
-   (set (match_dup 4)
-	(lshiftrt:DI (match_dup 4)
+   (set (match_dup 0)
+	(lshiftrt:DI (match_dup 0)
 		     (const_int 32)))
 
    ;; Shift OP0 into place.
-   (set (match_dup 0)
-	(ashift:DI (match_dup 0)
+   (set (match_dup 4)
+	(ashift:DI (match_dup 4)
 		   (const_int 32)))
 
    ;; OR the two halves together
    (set (match_dup 0)
 	(ior:DI (match_dup 0)
 		(match_dup 4)))]
-  { operands[5] = gen_rtx_REG (DImode, LO_REGNUM); }
-  [(set_attr "type" "imul")
-   (set_attr "mode" "SI")
-   (set_attr "length" "24")])
+  { operands[5] = gen_rtx_REG (DImode, LO_REGNUM); })
 
 (define_insn "<u>mulsidi3_64bit_hilo"
   [(set (match_operand:TI 0 "register_operand" "=x")
Index: testsuite/gcc.target/mips/mult-1.c
===================================================================
--- testsuite/gcc.target/mips/mult-1.c	(revision 0)
+++ testsuite/gcc.target/mips/mult-1.c	(revision 0)
@@ -0,0 +1,14 @@
+/* For SI->DI widening multiplication we should use DINS to combine the two
+   halves.  */
+/* { dg-options "-O -mgp64 isa_rev>=2" } */
+/* { dg-final { scan-assembler "\tdins\t" } } */
+/* { dg-final { scan-assembler-not "\tdsll\t" } } */
+/* { dg-final { scan-assembler-not "\tdsrl\t" } } */
+/* { dg-final { scan-assembler-not "\tor\t" } } */
+
+NOMIPS16 unsigned long long
+f (unsigned int i, unsigned int j)
+{
+  i++;
+  return (unsigned long long) i * j;
+}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add MIPS64r2 support to <u>mulsidi3_64bit
  2009-10-25 10:49   ` Adam Nemet
@ 2009-11-03 21:29     ` Richard Sandiford
  2009-11-04  0:14       ` Adam Nemet
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2009-11-03 21:29 UTC (permalink / raw)
  To: Adam Nemet; +Cc: gcc-patches

I only noticed later that a couple of the comments need to be updated:

Adam Nemet <anemet@caviumnetworks.com> writes:
> +   ;; OP0 <- LO, OP4 <- HI
> +   (set (match_dup 0) (match_dup 5))
> +   (set (match_dup 4) (unspec:DI [(match_dup 3)] UNSPEC_MFHI))
>  
>     ;; Zero-extend OP4.
> -   (set (match_dup 4)
> -	(ashift:DI (match_dup 4)
> +   (set (match_dup 0)
> +	(ashift:DI (match_dup 0)
>  		   (const_int 32)))
> -   (set (match_dup 4)
> -	(lshiftrt:DI (match_dup 4)
> +   (set (match_dup 0)
> +	(lshiftrt:DI (match_dup 0)
>  		     (const_int 32)))
>  
>     ;; Shift OP0 into place.
> -   (set (match_dup 0)
> -	(ashift:DI (match_dup 0)
> +   (set (match_dup 4)
> +	(ashift:DI (match_dup 4)
>  		   (const_int 32)))

Here's what I applied.

Richard


gcc/
	* config/mips/mips.md: Fix typos.

Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	2009-11-03 21:28:08.000000000 +0000
+++ gcc/config/mips/mips.md	2009-11-03 21:28:08.000000000 +0000
@@ -1930,7 +1930,7 @@ (define_split
    (set (match_dup 0) (match_dup 5))
    (set (match_dup 4) (unspec:DI [(match_dup 3)] UNSPEC_MFHI))
 
-   ;; Zero-extend OP4.
+   ;; Zero-extend OP0.
    (set (match_dup 0)
 	(ashift:DI (match_dup 0)
 		   (const_int 32)))
@@ -1938,7 +1938,7 @@ (define_split
 	(lshiftrt:DI (match_dup 0)
 		     (const_int 32)))
 
-   ;; Shift OP0 into place.
+   ;; Shift OP4 into place.
    (set (match_dup 4)
 	(ashift:DI (match_dup 4)
 		   (const_int 32)))

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Add MIPS64r2 support to <u>mulsidi3_64bit
  2009-11-03 21:29     ` Richard Sandiford
@ 2009-11-04  0:14       ` Adam Nemet
  0 siblings, 0 replies; 6+ messages in thread
From: Adam Nemet @ 2009-11-04  0:14 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches

Richard Sandiford writes:
> I only noticed later that a couple of the comments need to be updated:

Oops, thanks!

Adam

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-11-04  0:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-23 19:30 [PATCH] Add MIPS64r2 support to <u>mulsidi3_64bit Adam Nemet
2009-10-24 10:06 ` Richard Sandiford
2009-10-24 15:54   ` Adam Nemet
2009-10-25 10:49   ` Adam Nemet
2009-11-03 21:29     ` Richard Sandiford
2009-11-04  0:14       ` Adam Nemet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).