public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch,avr] tweak sign extensions
@ 2014-10-23 13:54 Georg-Johann Lay
  2014-10-23 18:24 ` Denis Chertykov
  0 siblings, 1 reply; 4+ messages in thread
From: Georg-Johann Lay @ 2014-10-23 13:54 UTC (permalink / raw)
  To: GCC Patches; +Cc: Denis Chertykov

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

This optimization makes most sign-extensions one instruction shorter in the 
case when the source register may be clobbered and the register numbers are 
different.  Source and destination may overlap.

Ok for trunk?


Johann

gcc/
	* config/avr/avr.md (extendqihi2, extendqipsi2, extendqisi2)
	(extendhipsi2, extendhisi2): Optimize if source reg is unused
	after the insns and has different REGNO than destination.

[-- Attachment #2: tweak-extend.diff --]
[-- Type: text/x-patch, Size: 4055 bytes --]

Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md	(revision 216592)
+++ config/avr/avr.md	(working copy)
@@ -4174,9 +4174,14 @@ (define_insn "extendqihi2"
   [(set (match_operand:HI 0 "register_operand" "=r,r")
         (sign_extend:HI (match_operand:QI 1 "combine_pseudo_register_operand" "0,*r")))]
   ""
-  "@
-	clr %B0\;sbrc %0,7\;com %B0
-	mov %A0,%A1\;clr %B0\;sbrc %A0,7\;com %B0"
+  {
+    if (REGNO (operands[0]) == REGNO (operands[1]))
+      return "clr %B0\;sbrc %0,7\;com %B0";
+    else if (reg_unused_after (insn, operands[1]))
+      return "mov %A0,%1\;lsl %1\;sbc %B0,%B0";
+    else
+      return "mov %A0,%1\;clr %B0\;sbrc %A0,7\;com %B0";
+  }
   [(set_attr "length" "3,4")
    (set_attr "cc" "set_n,set_n")])
 
@@ -4184,9 +4189,14 @@ (define_insn "extendqipsi2"
   [(set (match_operand:PSI 0 "register_operand" "=r,r")
         (sign_extend:PSI (match_operand:QI 1 "combine_pseudo_register_operand" "0,*r")))]
   ""
-  "@
-	clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0
-	mov %A0,%A1\;clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0"
+  {
+    if (REGNO (operands[0]) == REGNO (operands[1]))
+      return "clr %B0\;sbrc %0,7\;com %B0\;mov %C0,%B0";
+    else if (reg_unused_after (insn, operands[1]))
+      return "mov %A0,%1\;lsl %1\;sbc %B0,%B0\;mov %C0,%B0";
+    else
+      return "mov %A0,%1\;clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0";
+  }
   [(set_attr "length" "4,5")
    (set_attr "cc" "set_n,set_n")])
 
@@ -4194,9 +4204,14 @@ (define_insn "extendqisi2"
   [(set (match_operand:SI 0 "register_operand" "=r,r")
         (sign_extend:SI (match_operand:QI 1 "combine_pseudo_register_operand" "0,*r")))]
   ""
-  "@
-	clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0\;mov %D0,%B0
-	mov %A0,%A1\;clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0\;mov %D0,%B0"
+  {
+    if (REGNO (operands[0]) == REGNO (operands[1]))
+      return "clr %B0\;sbrc %0,7\;com %B0\;mov %C0,%B0\;mov %D0,%B0";
+    else if (reg_unused_after (insn, operands[1]))
+      return "mov %A0,%1\;lsl %1\;sbc %B0,%B0\;mov %C0,%B0\;mov %D0,%B0";
+    else
+      return "mov %A0,%1\;clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0\;mov %D0,%B0";
+  }
   [(set_attr "length" "5,6")
    (set_attr "cc" "set_n,set_n")])
 
@@ -4204,10 +4219,16 @@ (define_insn "extendhipsi2"
   [(set (match_operand:PSI 0 "register_operand"                               "=r,r ,r")
         (sign_extend:PSI (match_operand:HI 1 "combine_pseudo_register_operand" "0,*r,*r")))]
   ""
-  "@
-	clr %C0\;sbrc %B0,7\;com %C0
-	mov %A0,%A1\;mov %B0,%B1\;clr %C0\;sbrc %B0,7\;com %C0
-	movw %A0,%A1\;clr %C0\;sbrc %B0,7\;com %C0"
+  {
+    if (REGNO (operands[0]) == REGNO (operands[1]))
+      return "clr %C0\;sbrc %B0,7\;com %C0";
+    output_asm_insn (AVR_HAVE_MOVW
+                     ? "movw %A0,%A1"
+                     : "mov %A0,%A1\n\tmov %B0,%B1", operands);
+    return (reg_unused_after (insn, operands[1])
+            ? "lsl %B1\;sbc %C0,%C0"
+            : "clr %C0\;sbrc %B0,7\;com %C0");
+  }
   [(set_attr "length" "3,5,4")
    (set_attr "isa" "*,mov,movw")
    (set_attr "cc" "set_n")])
@@ -4216,10 +4237,16 @@ (define_insn "extendhisi2"
   [(set (match_operand:SI 0 "register_operand"                               "=r,r ,r")
         (sign_extend:SI (match_operand:HI 1 "combine_pseudo_register_operand" "0,*r,*r")))]
   ""
-  "@
-	clr %C0\;sbrc %B0,7\;com %C0\;mov %D0,%C0
-	mov %A0,%A1\;mov %B0,%B1\;clr %C0\;sbrc %B0,7\;com %C0\;mov %D0,%C0
-	movw %A0,%A1\;clr %C0\;sbrc %B0,7\;com %C0\;mov %D0,%C0"
+  {
+    if (REGNO (operands[0]) == REGNO (operands[1]))
+      return "clr %C0\;sbrc %B0,7\;com %C0\;mov %D0,%C0";
+    output_asm_insn (AVR_HAVE_MOVW
+                     ? "movw %A0,%A1"
+                     : "mov %A0,%A1\n\tmov %B0,%B1", operands);
+    return (reg_unused_after (insn, operands[1])
+            ? "lsl %B1\;sbc %C0,%C0\;mov %D0,%C0"
+            : "clr %C0\;sbrc %B0,7\;com %C0\;mov %D0,%C0");
+  }
   [(set_attr "length" "4,6,5")
    (set_attr "isa" "*,mov,movw")
    (set_attr "cc" "set_n")])

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

* Re: [patch,avr] tweak sign extensions
  2014-10-23 13:54 [patch,avr] tweak sign extensions Georg-Johann Lay
@ 2014-10-23 18:24 ` Denis Chertykov
  2014-10-24 10:39   ` [patch,avr] tweak sign extensions, take #2 Georg-Johann Lay
  0 siblings, 1 reply; 4+ messages in thread
From: Denis Chertykov @ 2014-10-23 18:24 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: GCC Patches

2014-10-23 17:48 GMT+04:00 Georg-Johann Lay <avr@gjlay.de>:
> This optimization makes most sign-extensions one instruction shorter in the
> case when the source register may be clobbered and the register numbers are
> different.  Source and destination may overlap.
>
> Ok for trunk?
>
>
> Johann
>
> gcc/
>         * config/avr/avr.md (extendqihi2, extendqipsi2, extendqisi2)
>         (extendhipsi2, extendhisi2): Optimize if source reg is unused
>         after the insns and has different REGNO than destination.

Approved.

Denis.

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

* Re: [patch,avr] tweak sign extensions, take #2
  2014-10-23 18:24 ` Denis Chertykov
@ 2014-10-24 10:39   ` Georg-Johann Lay
  2014-10-24 14:03     ` Denis Chertykov
  0 siblings, 1 reply; 4+ messages in thread
From: Georg-Johann Lay @ 2014-10-24 10:39 UTC (permalink / raw)
  To: GCC Patches; +Cc: Denis Chertykov

[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]

Am 10/23/2014 08:16 PM schrieb Denis Chertykov:
>> This optimization makes most sign-extensions one instruction shorter in the
>> case when the source register may be clobbered and the register numbers are
>> different.  Source and destination may overlap.
>>
>> Ok for trunk?
>>
>> Johann
>>
>> gcc/
>>          * config/avr/avr.md (extendqihi2, extendqipsi2, extendqisi2)
>>          (extendhipsi2, extendhisi2): Optimize if source reg is unused
>>          after the insns and has different REGNO than destination.
>
> Approved.
>
> Denis.

Finally I switched to a solution that avoids all the ugly asm snippets and 
special casing, and which is exact w.r.t code size.  So allow me drop the patch 
from above and to propose this one for trunk.  Sorry for the inconvenience.

In any case it uses LSL/SBC idiom instead of the old CLR/SBRC/COM.


Johann

	* avr-protos.h (avr_out_sign_extend): New.
	* avr.c (avr_adjust_insn_length) [ADJUST_LEN_SEXT]: Handle.
	(avr_out_sign_extend): New function.
	* avr.md (extendqihi2, extendqipsi2, extendqisi2, extendhipsi2)
	(extendhisi2, extendpsisi2): Use it.
	(adjust_len) [sext]: New.




[-- Attachment #2: tweak-extend-2.diff --]
[-- Type: text/x-patch, Size: 7029 bytes --]

Index: config/avr/avr-protos.h
===================================================================
--- config/avr/avr-protos.h	(revision 216591)
+++ config/avr/avr-protos.h	(working copy)
@@ -57,6 +57,7 @@ extern const char *avr_out_compare (rtx_
 extern const char *avr_out_compare64 (rtx_insn *, rtx*, int*);
 extern const char *ret_cond_branch (rtx x, int len, int reverse);
 extern const char *avr_out_movpsi (rtx_insn *, rtx*, int*);
+extern const char *avr_out_sign_extend (rtx_insn *, rtx*, int*);
 
 extern const char *ashlqi3_out (rtx_insn *insn, rtx operands[], int *len);
 extern const char *ashlhi3_out (rtx_insn *insn, rtx operands[], int *len);
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c	(revision 216592)
+++ config/avr/avr.c	(working copy)
@@ -7734,6 +7734,56 @@ avr_out_bitop (rtx insn, rtx *xop, int *
 }
 
 
+/* Output sign extension from XOP[1] to XOP[0] and return "".
+   If PLEN == NULL, print assembler instructions to perform the operation;
+   otherwise, set *PLEN to the length of the instruction sequence (in words)
+   as printed with PLEN == NULL.  */
+
+const char*
+avr_out_sign_extend (rtx_insn *insn, rtx *xop, int *plen)
+{
+  // Size in bytes of source resp. destination operand.
+  unsigned n_src = GET_MODE_SIZE (GET_MODE (xop[1]));
+  unsigned n_dest = GET_MODE_SIZE (GET_MODE (xop[0]));
+  rtx r_msb = all_regs_rtx[REGNO (xop[1]) + n_src - 1];
+
+  if (plen)
+    *plen = 0;
+
+  // Copy destination to source
+
+  if (REGNO (xop[0]) != REGNO (xop[1]))
+    {
+      gcc_assert (n_src <= 2);
+
+      if (n_src == 2)
+        avr_asm_len (AVR_HAVE_MOVW
+                     ? "movw %0,%1"
+                     : "mov %B0,%B1", xop, plen, 1);
+      if (n_src == 1 || !AVR_HAVE_MOVW)
+        avr_asm_len ("mov %A0,%A1", xop, plen, 1);
+    }
+
+  // Set Carry to the sign bit MSB.7...
+
+  if (REGNO (xop[0]) == REGNO (xop[1])
+      || !reg_unused_after (insn, r_msb))
+    {
+      avr_asm_len ("mov __tmp_reg__,%0", &r_msb, plen, 1);
+      r_msb = tmp_reg_rtx;
+    }
+  
+  avr_asm_len ("lsl %0", &r_msb, plen, 1);
+                   
+  // ...and propagate it to all the new sign bits
+
+  for (unsigned n = n_src; n < n_dest; n++)
+    avr_asm_len ("sbc %0,%0", &all_regs_rtx[REGNO (xop[0]) + n], plen, 1);
+
+  return "";
+}
+
+
 /* PLEN == NULL: Output code to add CONST_INT OP[0] to SP.
    PLEN != NULL: Set *PLEN to the length of that sequence.
    Return "".  */
@@ -8578,6 +8628,7 @@ avr_adjust_insn_length (rtx_insn *insn,
     case ADJUST_LEN_MOVMEM: avr_out_movmem (insn, op, &len); break;
     case ADJUST_LEN_XLOAD: avr_out_xload (insn, op, &len); break;
     case ADJUST_LEN_LPM: avr_out_lpm (insn, op, &len); break;
+    case ADJUST_LEN_SEXT: avr_out_sign_extend (insn, op, &len); break;
 
     case ADJUST_LEN_SFRACT: avr_out_fract (insn, op, true, &len); break;
     case ADJUST_LEN_UFRACT: avr_out_fract (insn, op, false, &len); break;
Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md	(revision 216592)
+++ config/avr/avr.md	(working copy)
@@ -147,7 +147,7 @@ (define_attr "length" ""
 ;; Otherwise do special processing depending on the attribute.
 
 (define_attr "adjust_len"
-  "out_bitop, plus, addto_sp,
+  "out_bitop, plus, addto_sp, sext,
    tsthi, tstpsi, tstsi, compare, compare64, call,
    mov8, mov16, mov24, mov32, reload_in16, reload_in24, reload_in32,
    ufract, sfract, round,
@@ -4174,62 +4174,66 @@ (define_insn "extendqihi2"
   [(set (match_operand:HI 0 "register_operand" "=r,r")
         (sign_extend:HI (match_operand:QI 1 "combine_pseudo_register_operand" "0,*r")))]
   ""
-  "@
-	clr %B0\;sbrc %0,7\;com %B0
-	mov %A0,%A1\;clr %B0\;sbrc %A0,7\;com %B0"
+  {
+    return avr_out_sign_extend (insn, operands, NULL);
+  }
   [(set_attr "length" "3,4")
-   (set_attr "cc" "set_n,set_n")])
+   (set_attr "adjust_len" "sext")
+   (set_attr "cc" "set_n")])
 
 (define_insn "extendqipsi2"
   [(set (match_operand:PSI 0 "register_operand" "=r,r")
         (sign_extend:PSI (match_operand:QI 1 "combine_pseudo_register_operand" "0,*r")))]
   ""
-  "@
-	clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0
-	mov %A0,%A1\;clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0"
+  {
+    return avr_out_sign_extend (insn, operands, NULL);
+  }
   [(set_attr "length" "4,5")
-   (set_attr "cc" "set_n,set_n")])
+   (set_attr "adjust_len" "sext")
+   (set_attr "cc" "set_n")])
 
 (define_insn "extendqisi2"
   [(set (match_operand:SI 0 "register_operand" "=r,r")
         (sign_extend:SI (match_operand:QI 1 "combine_pseudo_register_operand" "0,*r")))]
   ""
-  "@
-	clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0\;mov %D0,%B0
-	mov %A0,%A1\;clr %B0\;sbrc %A0,7\;com %B0\;mov %C0,%B0\;mov %D0,%B0"
+  {
+    return avr_out_sign_extend (insn, operands, NULL);
+  }
   [(set_attr "length" "5,6")
-   (set_attr "cc" "set_n,set_n")])
+   (set_attr "adjust_len" "sext")
+   (set_attr "cc" "set_n")])
 
 (define_insn "extendhipsi2"
-  [(set (match_operand:PSI 0 "register_operand"                               "=r,r ,r")
-        (sign_extend:PSI (match_operand:HI 1 "combine_pseudo_register_operand" "0,*r,*r")))]
+  [(set (match_operand:PSI 0 "register_operand"                               "=r,r")
+        (sign_extend:PSI (match_operand:HI 1 "combine_pseudo_register_operand" "0,*r")))]
   ""
-  "@
-	clr %C0\;sbrc %B0,7\;com %C0
-	mov %A0,%A1\;mov %B0,%B1\;clr %C0\;sbrc %B0,7\;com %C0
-	movw %A0,%A1\;clr %C0\;sbrc %B0,7\;com %C0"
-  [(set_attr "length" "3,5,4")
-   (set_attr "isa" "*,mov,movw")
+  {
+    return avr_out_sign_extend (insn, operands, NULL);
+  }
+  [(set_attr "length" "3,5")
+   (set_attr "adjust_len" "sext")
    (set_attr "cc" "set_n")])
 
 (define_insn "extendhisi2"
-  [(set (match_operand:SI 0 "register_operand"                               "=r,r ,r")
-        (sign_extend:SI (match_operand:HI 1 "combine_pseudo_register_operand" "0,*r,*r")))]
+  [(set (match_operand:SI 0 "register_operand"                               "=r,r")
+        (sign_extend:SI (match_operand:HI 1 "combine_pseudo_register_operand" "0,*r")))]
   ""
-  "@
-	clr %C0\;sbrc %B0,7\;com %C0\;mov %D0,%C0
-	mov %A0,%A1\;mov %B0,%B1\;clr %C0\;sbrc %B0,7\;com %C0\;mov %D0,%C0
-	movw %A0,%A1\;clr %C0\;sbrc %B0,7\;com %C0\;mov %D0,%C0"
-  [(set_attr "length" "4,6,5")
-   (set_attr "isa" "*,mov,movw")
+  {
+    return avr_out_sign_extend (insn, operands, NULL);
+  }
+  [(set_attr "length" "4,6")
+   (set_attr "adjust_len" "sext")
    (set_attr "cc" "set_n")])
 
 (define_insn "extendpsisi2"
   [(set (match_operand:SI 0 "register_operand"                                "=r")
         (sign_extend:SI (match_operand:PSI 1 "combine_pseudo_register_operand" "0")))]
   ""
-  "clr %D0\;sbrc %C0,7\;com %D0"
+  {
+    return avr_out_sign_extend (insn, operands, NULL);
+  }
   [(set_attr "length" "3")
+   (set_attr "adjust_len" "sext")
    (set_attr "cc" "set_n")])
 
 ;; xx<---x xx<---x xx<---x xx<---x xx<---x xx<---x xx<---x xx<---x xx<---x

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

* Re: [patch,avr] tweak sign extensions, take #2
  2014-10-24 10:39   ` [patch,avr] tweak sign extensions, take #2 Georg-Johann Lay
@ 2014-10-24 14:03     ` Denis Chertykov
  0 siblings, 0 replies; 4+ messages in thread
From: Denis Chertykov @ 2014-10-24 14:03 UTC (permalink / raw)
  To: Georg-Johann Lay; +Cc: GCC Patches

2014-10-24 14:37 GMT+04:00 Georg-Johann Lay <avr@gjlay.de>:
> Am 10/23/2014 08:16 PM schrieb Denis Chertykov:
>>>
>>> This optimization makes most sign-extensions one instruction shorter in
>>> the
>>> case when the source register may be clobbered and the register numbers
>>> are
>>> different.  Source and destination may overlap.
>>>
>>> Ok for trunk?
>>>
>>> Johann
>>>
>>> gcc/
>>>          * config/avr/avr.md (extendqihi2, extendqipsi2, extendqisi2)
>>>          (extendhipsi2, extendhisi2): Optimize if source reg is unused
>>>          after the insns and has different REGNO than destination.
>>
>>
>> Approved.
>>
>> Denis.
>
>
> Finally I switched to a solution that avoids all the ugly asm snippets and
> special casing, and which is exact w.r.t code size.  So allow me drop the
> patch from above and to propose this one for trunk.  Sorry for the
> inconvenience.
>
> In any case it uses LSL/SBC idiom instead of the old CLR/SBRC/COM.
>
>
> Johann
>
>         * avr-protos.h (avr_out_sign_extend): New.
>         * avr.c (avr_adjust_insn_length) [ADJUST_LEN_SEXT]: Handle.
>         (avr_out_sign_extend): New function.
>         * avr.md (extendqihi2, extendqipsi2, extendqisi2, extendhipsi2)
>         (extendhisi2, extendpsisi2): Use it.
>         (adjust_len) [sext]: New.
>
>
>

I'm agree with you. It's better.
Approved.

Denis.

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

end of thread, other threads:[~2014-10-24 13:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-23 13:54 [patch,avr] tweak sign extensions Georg-Johann Lay
2014-10-23 18:24 ` Denis Chertykov
2014-10-24 10:39   ` [patch,avr] tweak sign extensions, take #2 Georg-Johann Lay
2014-10-24 14:03     ` Denis Chertykov

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).