diff mbox

[avr] tweak sign extensions, take #2

Message ID 544A2BEE.4030106@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay Oct. 24, 2014, 10:37 a.m. UTC
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.

Comments

Denis Chertykov Oct. 24, 2014, 1:58 p.m. UTC | #1
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.
diff mbox

Patch

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