* [Patch,AVR]: Clean-up some SP insns
@ 2011-09-27 9:26 Georg-Johann Lay
2011-09-27 16:55 ` Richard Henderson
2011-10-05 10:09 ` Ping #1: " Georg-Johann Lay
0 siblings, 2 replies; 5+ messages in thread
From: Georg-Johann Lay @ 2011-09-27 9:26 UTC (permalink / raw)
To: gcc-patches; +Cc: Denis Chertykov, Eric Weddington
[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]
This is just a code clean-up.
The bulky code from *addhi3_sp_R_pc2 and *addhi3_sp_R_pc3 is done by a small C
function that does the same (except that it prints some comment depending on
-dp or -fverbose-asm).
*movhi_sp is an insn that should not be there and go away because it is a move
insn and there should be at most one move insn per mode (HImode in this case).
stack_register_operand, i.e. (REG:HI 32), is in register class STACK_REG, thus
not in NO_REGS, thus element of register_operand, general_regs and
nonimmediate_operand which are the predicates/condition of *movhi. *movhi
already knows to handle "q,r" and "r,q" moves, same applies to the output
function output_movhi.
The patch passes the test suite, of course.
Ok?
Moreover, I'd like to remove constraint "R" which is just used on one insn now
and replace it by 3-letter constraint C.. so that prefix R is free.
R is of absolutely no use in inline assembly and the constraint can be
renamed/removed from documentation, IMO.
Johann
* config/avr/avr-protos.h (avr_out_addto_sp): New prototype.
* config/avr/avr.c (avr_out_addto_sp): New function.
(adjust_insn_length): Handle ADJUST_LEN_ADDTO_SP.
* config/avr/avr.md (adjust_len): Add "addto_sp".
(*movhi_sp): Remove insn.
(*addhi3_sp_R_pc2, *addhi3_sp_R_pc3): Merge to *addhi3_sp_R.
[-- Attachment #2: cleanup-sp.diff --]
[-- Type: text/x-patch, Size: 9251 bytes --]
Index: config/avr/avr.md
===================================================================
--- config/avr/avr.md (revision 179206)
+++ config/avr/avr.md (working copy)
@@ -136,7 +136,7 @@ (define_attr "length" ""
;; Otherwise do special processing depending on the attribute.
(define_attr "adjust_len"
- "out_bitop, out_plus, tsthi, tstsi, compare,
+ "out_bitop, out_plus, addto_sp, tsthi, tstsi, compare,
mov8, mov16, mov32, reload_in16, reload_in32,
ashlqi, ashrqi, lshrqi,
ashlhi, ashrhi, lshrhi,
@@ -346,15 +346,6 @@ (define_expand "movhi"
}
}")
-(define_insn "*movhi_sp"
- [(set (match_operand:HI 0 "register_operand" "=q,r")
- (match_operand:HI 1 "register_operand" "r,q"))]
- "((stack_register_operand(operands[0], HImode) && register_operand (operands[1], HImode))
- || (register_operand (operands[0], HImode) && stack_register_operand(operands[1], HImode)))"
- "* return output_movhi (insn, operands, NULL);"
- [(set_attr "length" "5,2")
- (set_attr "cc" "none,none")])
-
(define_insn "movhi_sp_r_irq_off"
[(set (match_operand:HI 0 "stack_register_operand" "=q")
(unspec_volatile:HI [(match_operand:HI 1 "register_operand" "r")]
@@ -767,142 +758,16 @@ (define_insn "*addhi3_zero_extend1"
[(set_attr "length" "2")
(set_attr "cc" "set_n")])
-(define_insn "*addhi3_sp_R_pc2"
+(define_insn "*addhi3_sp_R"
[(set (match_operand:HI 1 "stack_register_operand" "=q")
(plus:HI (match_operand:HI 2 "stack_register_operand" "q")
(match_operand:HI 0 "avr_sp_immediate_operand" "R")))]
- "AVR_2_BYTE_PC"
- "*{
- if (CONST_INT_P (operands[0]))
- {
- switch(INTVAL (operands[0]))
- {
- case -6:
- return \"rcall .\" CR_TAB
- \"rcall .\" CR_TAB
- \"rcall .\";
- case -5:
- return \"rcall .\" CR_TAB
- \"rcall .\" CR_TAB
- \"push __tmp_reg__\";
- case -4:
- return \"rcall .\" CR_TAB
- \"rcall .\";
- case -3:
- return \"rcall .\" CR_TAB
- \"push __tmp_reg__\";
- case -2:
- return \"rcall .\";
- case -1:
- return \"push __tmp_reg__\";
- case 0:
- return \"\";
- case 1:
- return \"pop __tmp_reg__\";
- case 2:
- return \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\";
- case 3:
- return \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\";
- case 4:
- return \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\";
- case 5:
- return \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\";
- }
- }
- return \"bug\";
- }"
- [(set (attr "length")
- (cond [(eq (const_int -6) (symbol_ref "INTVAL (operands[0])")) (const_int 3)
- (eq (const_int -5) (symbol_ref "INTVAL (operands[0])")) (const_int 3)
- (eq (const_int -4) (symbol_ref "INTVAL (operands[0])")) (const_int 2)
- (eq (const_int -3) (symbol_ref "INTVAL (operands[0])")) (const_int 2)
- (eq (const_int -2) (symbol_ref "INTVAL (operands[0])")) (const_int 1)
- (eq (const_int -1) (symbol_ref "INTVAL (operands[0])")) (const_int 1)
- (eq (const_int 0) (symbol_ref "INTVAL (operands[0])")) (const_int 0)
- (eq (const_int 1) (symbol_ref "INTVAL (operands[0])")) (const_int 1)
- (eq (const_int 2) (symbol_ref "INTVAL (operands[0])")) (const_int 2)
- (eq (const_int 3) (symbol_ref "INTVAL (operands[0])")) (const_int 3)
- (eq (const_int 4) (symbol_ref "INTVAL (operands[0])")) (const_int 4)
- (eq (const_int 5) (symbol_ref "INTVAL (operands[0])")) (const_int 5)]
- (const_int 0)))])
-
-(define_insn "*addhi3_sp_R_pc3"
- [(set (match_operand:HI 1 "stack_register_operand" "=q")
- (plus:HI (match_operand:HI 2 "stack_register_operand" "q")
- (match_operand:QI 0 "avr_sp_immediate_operand" "R")))]
- "AVR_3_BYTE_PC"
- "*{
- if (CONST_INT_P (operands[0]))
- {
- switch(INTVAL (operands[0]))
- {
- case -6:
- return \"rcall .\" CR_TAB
- \"rcall .\";
- case -5:
- return \"rcall .\" CR_TAB
- \"push __tmp_reg__\" CR_TAB
- \"push __tmp_reg__\";
- case -4:
- return \"rcall .\" CR_TAB
- \"push __tmp_reg__\";
- case -3:
- return \"rcall .\";
- case -2:
- return \"push __tmp_reg__\" CR_TAB
- \"push __tmp_reg__\";
- case -1:
- return \"push __tmp_reg__\";
- case 0:
- return \"\";
- case 1:
- return \"pop __tmp_reg__\";
- case 2:
- return \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\";
- case 3:
- return \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\";
- case 4:
- return \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\";
- case 5:
- return \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\" CR_TAB
- \"pop __tmp_reg__\";
- }
- }
- return \"bug\";
- }"
- [(set (attr "length")
- (cond [(eq (const_int -6) (symbol_ref "INTVAL (operands[0])")) (const_int 2)
- (eq (const_int -5) (symbol_ref "INTVAL (operands[0])")) (const_int 3)
- (eq (const_int -4) (symbol_ref "INTVAL (operands[0])")) (const_int 2)
- (eq (const_int -3) (symbol_ref "INTVAL (operands[0])")) (const_int 1)
- (eq (const_int -2) (symbol_ref "INTVAL (operands[0])")) (const_int 2)
- (eq (const_int -1) (symbol_ref "INTVAL (operands[0])")) (const_int 1)
- (eq (const_int 0) (symbol_ref "INTVAL (operands[0])")) (const_int 0)
- (eq (const_int 1) (symbol_ref "INTVAL (operands[0])")) (const_int 1)
- (eq (const_int 2) (symbol_ref "INTVAL (operands[0])")) (const_int 2)
- (eq (const_int 3) (symbol_ref "INTVAL (operands[0])")) (const_int 3)
- (eq (const_int 4) (symbol_ref "INTVAL (operands[0])")) (const_int 4)
- (eq (const_int 5) (symbol_ref "INTVAL (operands[0])")) (const_int 5)]
- (const_int 0)))])
+ ""
+ {
+ return avr_out_addto_sp (operands, NULL);
+ }
+ [(set_attr "length" "5")
+ (set_attr "adjust_len" "addto_sp")])
(define_insn "*addhi3"
[(set (match_operand:HI 0 "register_operand" "=r,!w,!w,d,r,r")
Index: config/avr/avr-protos.h
===================================================================
--- config/avr/avr-protos.h (revision 179205)
+++ config/avr/avr-protos.h (working copy)
@@ -83,6 +83,7 @@ extern void avr_output_addr_vec_elt (FIL
extern const char *avr_out_sbxx_branch (rtx insn, rtx operands[]);
extern const char* avr_out_bitop (rtx, rtx*, int*);
extern const char* avr_out_plus (rtx*, int*);
+extern const char* avr_out_addto_sp (rtx*, int*);
extern bool avr_popcount_each_byte (rtx, int, int);
extern int extra_constraint_Q (rtx x);
Index: config/avr/avr.c
===================================================================
--- config/avr/avr.c (revision 179206)
+++ config/avr/avr.c (working copy)
@@ -4896,6 +4896,47 @@ avr_out_bitop (rtx insn, rtx *xop, int *
return "";
}
+
+/* PLEN == NULL: Output code to add CONST_INT OP[0] to SP.
+ PLEN != NULL: Set *PLEN to the length of that sequence.
+ Return "". */
+
+const char*
+avr_out_addto_sp (rtx *op, int *plen)
+{
+ int pc_len = AVR_2_BYTE_PC ? 2 : 3;
+ int addend = INTVAL (op[0]);
+
+ if (plen)
+ *plen = 0;
+
+ if (addend < 0)
+ {
+ if (flag_verbose_asm || flag_print_asm_name)
+ avr_asm_len (ASM_COMMENT_START "SP -= %n0", op, plen, 0);
+
+ while (addend <= -pc_len)
+ {
+ addend += pc_len;
+ avr_asm_len ("rcall .", op, plen, 1);
+ }
+
+ while (addend++ < 0)
+ avr_asm_len ("push __zero_reg__", op, plen, 1);
+ }
+ else if (addend > 0)
+ {
+ if (flag_verbose_asm || flag_print_asm_name)
+ avr_asm_len (ASM_COMMENT_START "SP += %0", op, plen, 0);
+
+ while (addend-- > 0)
+ avr_asm_len ("pop __tmp_reg__", op, plen, 1);
+ }
+
+ return "";
+}
+
+
/* Create RTL split patterns for byte sized rotate expressions. This
produces a series of move instructions and considers overlap situations.
Overlapping non-HImode operands need a scratch register. */
@@ -5089,6 +5130,8 @@ adjust_insn_length (rtx insn, int len)
case ADJUST_LEN_OUT_BITOP: avr_out_bitop (insn, op, &len); break;
case ADJUST_LEN_OUT_PLUS: avr_out_plus (op, &len); break;
+
+ case ADJUST_LEN_ADDTO_SP: avr_out_addto_sp (op, &len); break;
case ADJUST_LEN_MOV8: output_movqi (insn, op, &len); break;
case ADJUST_LEN_MOV16: output_movhi (insn, op, &len); break;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch,AVR]: Clean-up some SP insns
2011-09-27 9:26 [Patch,AVR]: Clean-up some SP insns Georg-Johann Lay
@ 2011-09-27 16:55 ` Richard Henderson
2011-09-27 21:30 ` Georg-Johann Lay
2011-10-05 10:09 ` Ping #1: " Georg-Johann Lay
1 sibling, 1 reply; 5+ messages in thread
From: Richard Henderson @ 2011-09-27 16:55 UTC (permalink / raw)
To: Georg-Johann Lay; +Cc: gcc-patches, Denis Chertykov, Eric Weddington
On 09/27/2011 12:56 AM, Georg-Johann Lay wrote:
> * config/avr/avr-protos.h (avr_out_addto_sp): New prototype.
> * config/avr/avr.c (avr_out_addto_sp): New function.
> (adjust_insn_length): Handle ADJUST_LEN_ADDTO_SP.
> * config/avr/avr.md (adjust_len): Add "addto_sp".
> (*movhi_sp): Remove insn.
> (*addhi3_sp_R_pc2, *addhi3_sp_R_pc3): Merge to *addhi3_sp_R.
While this is a good cleanup by itself, and probably should be
applied by itself, I think a good followup would be to merge
the addhi3_sp_R pattern with the normal addhi3 pattern.
This should be fairly easy, given that both are handled via C
functions now.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch,AVR]: Clean-up some SP insns
2011-09-27 16:55 ` Richard Henderson
@ 2011-09-27 21:30 ` Georg-Johann Lay
0 siblings, 0 replies; 5+ messages in thread
From: Georg-Johann Lay @ 2011-09-27 21:30 UTC (permalink / raw)
To: Richard Henderson; +Cc: gcc-patches, Denis Chertykov, Eric Weddington
Richard Henderson schrieb:
> On 09/27/2011 12:56 AM, Georg-Johann Lay wrote:
>
>> * config/avr/avr-protos.h (avr_out_addto_sp): New prototype.
>> * config/avr/avr.c (avr_out_addto_sp): New function.
>> (adjust_insn_length): Handle ADJUST_LEN_ADDTO_SP.
>> * config/avr/avr.md (adjust_len): Add "addto_sp".
>> (*movhi_sp): Remove insn.
>> (*addhi3_sp_R_pc2, *addhi3_sp_R_pc3): Merge to *addhi3_sp_R.
>
> While this is a good cleanup by itself, and probably should be
> applied by itself, I think a good followup would be to merge
> the addhi3_sp_R pattern with the normal addhi3 pattern.
>
> This should be fairly easy, given that both are handled via C
> functions now.
There is bit more work for *addhi3 (PR50447) and thus adapting
prologue/epilogue functions etc. Didn't do that yet.
Johann
> r~
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Ping #1: [Patch,AVR]: Clean-up some SP insns
2011-09-27 9:26 [Patch,AVR]: Clean-up some SP insns Georg-Johann Lay
2011-09-27 16:55 ` Richard Henderson
@ 2011-10-05 10:09 ` Georg-Johann Lay
2011-10-05 10:43 ` Denis Chertykov
1 sibling, 1 reply; 5+ messages in thread
From: Georg-Johann Lay @ 2011-10-05 10:09 UTC (permalink / raw)
To: gcc-patches; +Cc: Denis Chertykov, Eric Weddington
Ping #1: http://gcc.gnu.org/ml/gcc-patches/2011-09/msg01690.html
Georg-Johann Lay wrote:
> This is just a code clean-up.
>
> The bulky code from *addhi3_sp_R_pc2 and *addhi3_sp_R_pc3 is done by a small C
> function that does the same (except that it prints some comment depending on
> -dp or -fverbose-asm).
>
> *movhi_sp is an insn that should not be there and go away because it is a move
> insn and there should be at most one move insn per mode (HImode in this case).
>
> stack_register_operand, i.e. (REG:HI 32), is in register class STACK_REG, thus
> not in NO_REGS, thus element of register_operand, general_regs and
> nonimmediate_operand which are the predicates/condition of *movhi. *movhi
> already knows to handle "q,r" and "r,q" moves, same applies to the output
> function output_movhi.
>
> The patch passes the test suite, of course.
>
> Ok?
>
> Moreover, I'd like to remove constraint "R" which is just used in one insn now
> and replace it by 3-letter constraint C.. so that prefix R is free.
>
> R is of absolutely no use in inline assembly and the constraint can be
> renamed/removed from documentation, IMO.
>
> Johann
>
> * config/avr/avr-protos.h (avr_out_addto_sp): New prototype.
> * config/avr/avr.c (avr_out_addto_sp): New function.
> (adjust_insn_length): Handle ADJUST_LEN_ADDTO_SP.
> * config/avr/avr.md (adjust_len): Add "addto_sp".
> (*movhi_sp): Remove insn.
> (*addhi3_sp_R_pc2, *addhi3_sp_R_pc3): Merge to *addhi3_sp_R.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Ping #1: [Patch,AVR]: Clean-up some SP insns
2011-10-05 10:09 ` Ping #1: " Georg-Johann Lay
@ 2011-10-05 10:43 ` Denis Chertykov
0 siblings, 0 replies; 5+ messages in thread
From: Denis Chertykov @ 2011-10-05 10:43 UTC (permalink / raw)
To: Georg-Johann Lay; +Cc: gcc-patches, Eric Weddington
2011/10/5 Georg-Johann Lay <avr@gjlay.de>:
>
> Ping #1: http://gcc.gnu.org/ml/gcc-patches/2011-09/msg01690.html
>
> Georg-Johann Lay wrote:
>
>> This is just a code clean-up.
>>
>> The bulky code from *addhi3_sp_R_pc2 and *addhi3_sp_R_pc3 is done by a small C
>> function that does the same (except that it prints some comment depending on
>> -dp or -fverbose-asm).
>>
>> *movhi_sp is an insn that should not be there and go away because it is a move
>> insn and there should be at most one move insn per mode (HImode in this case).
>>
>> stack_register_operand, i.e. (REG:HI 32), is in register class STACK_REG, thus
>> not in NO_REGS, thus element of register_operand, general_regs and
>> nonimmediate_operand which are the predicates/condition of *movhi. *movhi
>> already knows to handle "q,r" and "r,q" moves, same applies to the output
>> function output_movhi.
>>
>> The patch passes the test suite, of course.
>>
>> Ok?
>>
>> Moreover, I'd like to remove constraint "R" which is just used in one insn now
>> and replace it by 3-letter constraint C.. so that prefix R is free.
>>
>> R is of absolutely no use in inline assembly and the constraint can be
>> renamed/removed from documentation, IMO.
>>
>> Johann
>>
>> * config/avr/avr-protos.h (avr_out_addto_sp): New prototype.
>> * config/avr/avr.c (avr_out_addto_sp): New function.
>> (adjust_insn_length): Handle ADJUST_LEN_ADDTO_SP.
>> * config/avr/avr.md (adjust_len): Add "addto_sp".
>> (*movhi_sp): Remove insn.
>> (*addhi3_sp_R_pc2, *addhi3_sp_R_pc3): Merge to *addhi3_sp_R.
Please commit.
Denis.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-10-05 10:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-27 9:26 [Patch,AVR]: Clean-up some SP insns Georg-Johann Lay
2011-09-27 16:55 ` Richard Henderson
2011-09-27 21:30 ` Georg-Johann Lay
2011-10-05 10:09 ` Ping #1: " Georg-Johann Lay
2011-10-05 10:43 ` 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).