diff mbox

Power: Reorder a sign-extend RTL pattern for readability

Message ID alpine.DEB.1.10.1208101321280.20608@tp.orcam.me.uk
State Accepted
Headers show

Commit Message

Maciej W. Rozycki Aug. 10, 2012, 12:40 p.m. UTC 
Hi,

 While examining the Power MD file seeking the explanation for a problem I 
saw I have noticed a change in the past separated one of the instruction 
splitters from its corresponding instruction pattern.  Several unrelated 
patterns were inserted between the two, presumably by accident where the 
`patch' tool applied a diff made from an older or modified tree in the 
wrong place.

 I find this arrangement confusing, so I propose to move the splitter 
back, next to the other pattern.  Here's the intended update.  No 
functional change.  OK to apply?

2012-08-10  Maciej W. Rozycki  <macro@codesourcery.com>

	gcc/
	* config/rs6000/rs6000.md: Move a splitter next to its insn.

  Maciej

gcc-rs6000-sign-extend-reorder.diff

Comments

Maciej W. Rozycki Sept. 8, 2012, 12:27 a.m. UTC | #1 
Hi,

 This almost obviously correct proposal:

http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00619.html

received no feedback whatsoever and still awaits review.  Would you please 
have a look at it?  Thanks.

  Maciej
David Edelsohn Sept. 8, 2012, 12:28 p.m. UTC | #2 
While examining the Power MD file seeking the explanation for a problem I
saw I have noticed a change in the past separated one of the instruction
splitters from its corresponding instruction pattern.  Several unrelated
patterns were inserted between the two, presumably by accident where the
`patch' tool applied a diff made from an older or modified tree in the
wrong place.

 I find this arrangement confusing, so I propose to move the splitter
back, next to the other pattern.  Here's the intended update.  No
functional change.  OK to apply?

2012-08-10  Maciej W. Rozycki  <macro@codesourcery.com>

	gcc/
	* config/rs6000/rs6000.md: Move a splitter next to its insn.

This patch is okay.  Yes, the splitter should not have been separated
from the basic pattern. Thanks for helping to clean up the port.

Thanks, David
Maciej W. Rozycki Sept. 10, 2012, 9:09 p.m. UTC | #3 
On Sat, 8 Sep 2012, David Edelsohn wrote:

> 2012-08-10  Maciej W. Rozycki  <macro@codesourcery.com>
> 
> 	gcc/
> 	* config/rs6000/rs6000.md: Move a splitter next to its insn.
> 
> This patch is okay.  Yes, the splitter should not have been separated
> from the basic pattern. Thanks for helping to clean up the port.

 Applied now, thanks for your review.

  Maciej
diff mbox

Patch

Index: gcc-fsf-trunk-quilt/gcc/config/rs6000/rs6000.md
===================================================================
--- gcc-fsf-trunk-quilt.orig/gcc/config/rs6000/rs6000.md	2012-05-13 13:50:31.000000000 +0100
+++ gcc-fsf-trunk-quilt/gcc/config/rs6000/rs6000.md	2012-08-10 03:13:05.030525435 +0100
@@ -1048,6 +1048,20 @@ 
    #"
   [(set_attr "type" "compare")
    (set_attr "length" "4,8")])
+
+(define_split
+  [(set (match_operand:CC 2 "cc_reg_not_micro_cr0_operand" "")
+	(compare:CC (sign_extend:SI (match_operand:HI 1 "gpc_reg_operand" ""))
+		    (const_int 0)))
+   (set (match_operand:SI 0 "gpc_reg_operand" "")
+	(sign_extend:SI (match_dup 1)))]
+  "reload_completed"
+  [(set (match_dup 0)
+	(sign_extend:SI (match_dup 1)))
+   (set (match_dup 2)
+	(compare:CC (match_dup 0)
+		    (const_int 0)))]
+  "")
 
 ;; IBM 405, 440, 464 and 476 half-word multiplication operations.
 
@@ -1580,20 +1594,6 @@ 
   DONE;
 })
 
-(define_split
-  [(set (match_operand:CC 2 "cc_reg_not_micro_cr0_operand" "")
-	(compare:CC (sign_extend:SI (match_operand:HI 1 "gpc_reg_operand" ""))
-		    (const_int 0)))
-   (set (match_operand:SI 0 "gpc_reg_operand" "")
-	(sign_extend:SI (match_dup 1)))]
-  "reload_completed"
-  [(set (match_dup 0)
-	(sign_extend:SI (match_dup 1)))
-   (set (match_dup 2)
-	(compare:CC (match_dup 0)
-		    (const_int 0)))]
-  "")
-
 ;; Fixed-point arithmetic insns.
 
 (define_expand "add<mode>3"