Patchwork Power: Reorder a sign-extend RTL pattern for readability

login
register
mail settings
Submitter Maciej W. Rozycki
Date Aug. 10, 2012, 12:40 p.m.
Message ID <alpine.DEB.1.10.1208101321280.20608@tp.orcam.me.uk>
Download mbox | patch
Permalink /patch/176484/
State New
Headers show

Comments

Maciej W. Rozycki - Aug. 10, 2012, 12:40 p.m.
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
Maciej W. Rozycki - Sept. 8, 2012, 12:27 a.m.
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.
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.
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

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"