diff mbox series

[2/2,MSP430] Optimize zero_extend insns and PSImode pointer manipulation

Message ID 20191008113957.054ec793@jozef-kubuntu
State New
Headers show
Series Optimize zero_extend insns and PSImode pointer manipulation | expand

Commit Message

Jozef Lawrynowicz Oct. 8, 2019, 10:39 a.m. UTC 
This patch has the functional changes to optimize zero_extend insns and pointer
manipulation in the large memory model.

Comments

Segher Boessenkool Oct. 9, 2019, 8:47 a.m. UTC | #1 
Hi Jozef,

On Tue, Oct 08, 2019 at 11:39:57AM +0100, Jozef Lawrynowicz wrote:
> +;; ------------------------
> +;; ZERO EXTEND INSTRUCTIONS
> +;; Byte-writes to registers clear bits 19:8
> +;;   * Byte-writes to memory do not affect bits 15:8
> +;; Word-writes to registers clear bits 19:16
> +;; PSImode writes to memory clear bits 16:4 of the second memory word

Should that be 15:4 instead?

> +; FIXME many of these should be unnnecessary once combine deals with
> +; (sign_extend (zero_extend)) or (sign_extend (subreg)) BZ 91865.

I have looked at it some time ago, and it is quite hard :-/  It's not
forgotten though!


Segher
Jozef Lawrynowicz Oct. 9, 2019, 1:42 p.m. UTC | #2 
Hi Segher!

On Wed, 9 Oct 2019 03:47:32 -0500
Segher Boessenkool <segher@kernel.crashing.org> wrote:

> Hi Jozef,
> 
> On Tue, Oct 08, 2019 at 11:39:57AM +0100, Jozef Lawrynowicz wrote:
> > +;; ------------------------
> > +;; ZERO EXTEND INSTRUCTIONS
> > +;; Byte-writes to registers clear bits 19:8
> > +;;   * Byte-writes to memory do not affect bits 15:8
> > +;; Word-writes to registers clear bits 19:16
> > +;; PSImode writes to memory clear bits 16:4 of the second memory word  
> 
> Should that be 15:4 instead?

Whoops, nice spot. I'll fix that before applying (if it gets approved) rather
than re-submitting.
> 
> > +; FIXME many of these should be unnnecessary once combine deals with
> > +; (sign_extend (zero_extend)) or (sign_extend (subreg)) BZ 91865.  
> 
> I have looked at it some time ago, and it is quite hard :-/  It's not
> forgotten though!

Ok, thanks for the update!
Jozef
> 
> 
> Segher
Jeff Law Oct. 14, 2019, 9:22 p.m. UTC | #3 
On 10/8/19 4:39 AM, Jozef Lawrynowicz wrote:
> This patch has the functional changes to optimize zero_extend insns and pointer
> manipulation in the large memory model.
> 
> 
> 0002-MSP430-PSImode-pointer-manipulation-and-zero-extend-.patch
> 
> From f8156e115c4743ce94a86835ffa5601b6d28a555 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Mon, 7 Oct 2019 11:44:16 +0100
> Subject: [PATCH 2/2] MSP430: PSImode pointer manipulation and zero extend insn
>  optimizations
> 
> gcc/ChangeLog:
> 
> 2019-10-08  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> 
> 	* config/msp430/msp430.md (movqipsi): New.
> 	(zero_extendqipsi2): New.
> 	(zero_extendqisi2): Optimize case where src register and base dst
> 	register are the same.
> 	(zero_extendhipsi2): Don't use 430X insn for rYs->r case.
> 	(zero_extendpsisi2): Optimize r->m case.
> 	Add unnamed insn patterns to catch insns combine searches for when
> 	optimizing pointer manipulation.
So you've got a movqipsi and zero_extendqipsi2 with the same RTL
structure.  ISTM the movqipsi should just get removed.


OK with that change.

jeff
Jozef Lawrynowicz Oct. 15, 2019, 12:50 p.m. UTC | #4 
On Mon, Oct 14, 2019 at 10:22 PM Jeff Law <law@redhat.com> wrote:
>
> On 10/8/19 4:39 AM, Jozef Lawrynowicz wrote:
> > This patch has the functional changes to optimize zero_extend insns and pointer
> > manipulation in the large memory model.
> >
> >
> > 0002-MSP430-PSImode-pointer-manipulation-and-zero-extend-.patch
> >
> > From f8156e115c4743ce94a86835ffa5601b6d28a555 Mon Sep 17 00:00:00 2001
> > From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> > Date: Mon, 7 Oct 2019 11:44:16 +0100
> > Subject: [PATCH 2/2] MSP430: PSImode pointer manipulation and zero extend insn
> >  optimizations
> >
> > gcc/ChangeLog:
> >
> > 2019-10-08  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
> >
> >       * config/msp430/msp430.md (movqipsi): New.
> >       (zero_extendqipsi2): New.
> >       (zero_extendqisi2): Optimize case where src register and base dst
> >       register are the same.
> >       (zero_extendhipsi2): Don't use 430X insn for rYs->r case.
> >       (zero_extendpsisi2): Optimize r->m case.
> >       Add unnamed insn patterns to catch insns combine searches for when
> >       optimizing pointer manipulation.
> So you've got a movqipsi and zero_extendqipsi2 with the same RTL
> structure.  ISTM the movqipsi should just get removed.

Thanks, fixed and applied.
Jozef
>
>
> OK with that change.
>
> jeff
diff mbox series

Patch

From f8156e115c4743ce94a86835ffa5601b6d28a555 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Mon, 7 Oct 2019 11:44:16 +0100
Subject: [PATCH 2/2] MSP430: PSImode pointer manipulation and zero extend insn
 optimizations

gcc/ChangeLog:

2019-10-08  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* config/msp430/msp430.md (movqipsi): New.
	(zero_extendqipsi2): New.
	(zero_extendqisi2): Optimize case where src register and base dst
	register are the same.
	(zero_extendhipsi2): Don't use 430X insn for rYs->r case.
	(zero_extendpsisi2): Optimize r->m case.
	Add unnamed insn patterns to catch insns combine searches for when
	optimizing pointer manipulation.
---
 gcc/config/msp430/msp430.md | 135 +++++++++++++++++++++++++++++++-----
 1 file changed, 117 insertions(+), 18 deletions(-)

diff --git a/gcc/config/msp430/msp430.md b/gcc/config/msp430/msp430.md
index 2e8e8326232..cb0b3f16dc5 100644
--- a/gcc/config/msp430/msp430.md
+++ b/gcc/config/msp430/msp430.md
@@ -182,6 +182,15 @@ 
    MOV%X1.B\t%1, %0"
 )
 
+(define_insn "movqipsi"
+  [(set (match_operand:PSI		   0 "register_operand" "=r,r")
+	(zero_extend:PSI (match_operand:QI 1 "general_operand" "rYs,m")))]
+  "msp430x"
+  "@
+   MOV.B\t%1, %0
+   MOV%X1.B\t%1, %0"
+)
+
 (define_insn "movqi_topbyte"
   [(set (match_operand:QI 0 "msp430_general_dst_operand" "=r")
 	(subreg:QI (match_operand:PSI 1 "msp430_general_operand" "r") 2))]
@@ -553,6 +562,16 @@ 
    SXT%X0\t%0"
 )
 
+;; ------------------------
+;; ZERO EXTEND INSTRUCTIONS
+;; Byte-writes to registers clear bits 19:8
+;;   * Byte-writes to memory do not affect bits 15:8
+;; Word-writes to registers clear bits 19:16
+;; PSImode writes to memory clear bits 16:4 of the second memory word
+;; We define all possible insns since that results in better code than if
+;; they are inferred.
+;; ------------------------
+
 (define_insn "zero_extendqihi2"
   [(set (match_operand:HI		  0 "msp430_general_dst_operand" "=rYs,r,r,m")
 	(zero_extend:HI (match_operand:QI 1 "msp430_general_operand" "0,rYs,m,0")))]
@@ -564,19 +583,31 @@ 
    AND%X0\t#0xff, %0"
 )
 
+(define_insn "zero_extendqipsi2"
+  [(set (match_operand:PSI		   0 "register_operand" "=r,r")
+	(zero_extend:PSI (match_operand:QI 1 "general_operand" "rYs,m")))]
+  "msp430x"
+  "@
+   MOV.B\t%1, %0
+   MOV%X1.B\t%1, %0"
+)
+
 (define_insn "zero_extendqisi2"
-  [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=r")
-	(zero_extend:SI (match_operand:QI 1 "nonimmediate_operand" "rm")))]
+  [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=r,r")
+	(zero_extend:SI (match_operand:QI 1 "nonimmediate_operand" "0,rm")))]
   ""
-  "MOV%X1.B\t%1,%L0 { CLR\t%H0"
+  "@
+  CLR\t%H0
+  MOV%X1.B\t%1,%L0 { CLR\t%H0"
 )
 
 (define_insn "zero_extendhipsi2"
-  [(set (match_operand:PSI		   0 "msp430_general_dst_operand" "=r,m")
-	(zero_extend:PSI (match_operand:HI 1 "msp430_general_operand" "rm,r")))]
-  ""
+  [(set (match_operand:PSI		   0 "msp430_general_dst_operand" "=r,r,m")
+	(zero_extend:PSI (match_operand:HI 1 "msp430_general_operand"     "rYs,m,r")))]
+  "msp430x"
   "@
-  MOVX\t%1, %0
+  MOV.W\t%1, %0
+  MOV%X1\t%1, %0
   MOVX.A\t%1, %0"
 )
 
@@ -616,22 +647,90 @@ 
 ; the pair is unused and so it can clobber it.  Try compiling 20050826-2.c
 ; at -O2 to see this.
 
+; FIXME we can use MOVA for r->m if m is &abs20 or z16(rdst)
 (define_insn "zero_extendpsisi2"
-  [(set (match_operand:SI                  0 "register_operand" "+r")
-	(zero_extend:SI (match_operand:PSI 1 "register_operand" "r")))]
+  [(set (match_operand:SI		   0 "register_operand" "+r,m")
+	(zero_extend:SI (match_operand:PSI 1 "register_operand" "r,r")))]
   ""
-  "*
-    if (REGNO (operands[1]) == SP_REGNO)
-      /* If the source register is the stack pointer, the value
-         stored in the stack slot will be the value *after* the
-	 stack pointer has been decremented.  So allow for that
-	 here.  */
-      return \"PUSHM.A\t#1, %1 { ADDX.W\t#4, @r1 { POPX.W\t%L0 { POPX.W\t%H0 ; get stack pointer into %L0:%H0\";
-    else
+  "@
+  * if (REGNO (operands[1]) == SP_REGNO) \
+      /* If the source register is the stack pointer, the value \
+	 stored in the stack slot will be the value *after* the \
+	 stack pointer has been decremented.  So allow for that \
+	 here.  */ \
+      return \"PUSHM.A\t#1, %1 { ADDX.W\t#4, @r1 { POPX.W\t%L0 { POPX.W\t%H0 ; get stack pointer into %L0:%H0\"; \
+    else \
       return \"PUSHM.A\t#1, %1 { POPX.W\t%L0 { POPX.W\t%H0 ; move pointer in %1 into reg-pair %L0:%H0\";
-  "
+  MOVX.A %1, %0"
+)
+
+;; Below are unnamed insn patterns to catch pointer manipulation insns
+;; generated by combine.
+;; We get large code size bloat when a PSImode pointer is stored in
+;; memory, so we try to avoid that where possible and keep point manipulation
+;; between registers.
+; FIXME many of these should be unnnecessary once combine deals with
+; (sign_extend (zero_extend)) or (sign_extend (subreg)) BZ 91865.
+
+;; This is just another way of writing movqipsi/zero_extendqipsi
+(define_insn ""
+  [(set (match_operand:PSI 0 "register_operand" "=r")
+	(sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0)))]
+  "msp430x"
+  "MOV%X1.B\t%1, %0"
+)
+
+(define_insn ""
+  [(set (match_operand:PSI				   0 "register_operand" "=r,r")
+	(sign_extend:PSI (zero_extend:HI (match_operand:QI 1 "general_operand" "rYs,m"))))]
+  "msp430x"
+  "@
+   MOV.B\t%1, %0
+   MOV%X1.B\t%1, %0"
+)
+
+(define_insn ""
+  [(set (match_operand:SI 0 "register_operand" "=r")
+	(ashift:SI (zero_extend:SI (match_operand:QI 1 "general_operand" "rm"))
+		   (match_operand:HI 2 "immediate_operand" "M")))]
+  "msp430x"
+  "MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0"
 )
 
+;; We are taking a char and shifting it and putting the result in 2 registers.
+;; the high register will always be for 0 shift counts < 8.
+(define_insn ""
+  [(set (match_operand:SI 0 "register_operand" "=r")
+	(ashift:SI (zero_extend:SI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0))
+		   (match_operand:HI 2 "immediate_operand" "M")))]
+  "msp430x"
+  "MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0"
+)
+
+;; Same as above but with a NOP sign_extend round the subreg
+(define_insn ""
+  [(set (match_operand:SI 0 "register_operand" "=r")
+	(ashift:SI (zero_extend:SI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0)))
+		   (match_operand:HI 2 "immediate_operand" "M")))]
+  "msp430x"
+  "MOV%X1.B %1, %L0 { RLAM.W %2, %L0 { CLR %H0"
+)
+
+(define_insn ""
+  [(set (match_operand:SI 0 "register_operand" "=r")
+	(zero_extend:SI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0))))]
+  "msp430x"
+  "MOV%X1.B %1, %L0 { CLR %H0"
+)
+
+(define_insn ""
+  [(set (match_operand:PSI 0 "register_operand" "=r")
+	(ashift:PSI (sign_extend:PSI (subreg:HI (match_operand:QI 1 "general_operand" "rm") 0))
+		    (match_operand:HI 2 "immediate_operand" "M")))]
+  "msp430x"
+  "MOV%X1.B %1, %0 { RLAM.W %2, %0"
+)
+;; END msp430 pointer manipulation combine insn patterns
 
 ;; Eliminate extraneous zero-extends mysteriously created by gcc.
 (define_peephole2
-- 
2.17.1