* [Patch, AVR]: Fix PR33049 (implement extzv) @ 2011-06-20 18:39 Georg-Johann Lay 2011-06-21 7:42 ` Denis Chertykov 0 siblings, 1 reply; 8+ messages in thread From: Georg-Johann Lay @ 2011-06-20 18:39 UTC (permalink / raw) Cc: gcc-patches, Denis Chertykov, Anatoly Sokolov, Eric B. Weddington [-- Attachment #1: Type: text/plain, Size: 642 bytes --] This is an optimization patch that implements extzv for 1-bit extracts. The nice thing is that AVR can do this easily with a BLD/CLR/BST sequence, without putting pressure on d-regs and without the requirement of source being in the same register as destination. extzv can also be seen in conjunction with zero_extend which, without this patch, will lead to a 16-bit loop just to get one bit from a position with a known offset. So there are two splits to split the high-part away. Tested without regression. Johann PR target/33049 * config/avr/avr.md (extzv): New expander. (*extzv, *extzv.qihi1, *extzv.qihi2): New insn-and-split. [-- Attachment #2: pr33049.diff --] [-- Type: text/x-patch, Size: 2929 bytes --] Index: config/avr/avr.md =================================================================== --- config/avr/avr.md (revision 175201) +++ config/avr/avr.md (working copy) @@ -3540,3 +3540,75 @@ (define_insn_and_split "*ior<mode>qi.byt int byteno = INTVAL(operands[2]) / BITS_PER_UNIT; operands[4] = simplify_gen_subreg (QImode, operands[0], <MODE>mode, byteno); }) + +(define_expand "extzv" + [(set (match_operand:QI 0 "register_operand" "") + (zero_extract:QI (match_operand:QI 1 "register_operand" "") + (match_operand:QI 2 "const1_operand" "") + (match_operand:QI 3 "const_0_to_7_operand" "")))] + "" + "") + +(define_insn_and_split "*extzv" + [(set (match_operand:QI 0 "register_operand" "=*d,*d,*d,r") + (zero_extract:QI (match_operand:QI 1 "register_operand" "0,r,0,r") + (const_int 1) + (match_operand:QI 2 "const_0_to_7_operand" "L,L,P,n")))] + "" + "@ + andi %0,1 + mov %0,%1\;andi %0,1 + lsr %0\;andi %0,1 + bst %1,%2\;clr %0\;bld %0,0" + "reload_completed + && INTVAL (operands[2]) == 4 + && REGNO (operands[0]) == REGNO (operands[1]) + && REGNO (operands[0]) >= 16" + [(set (match_dup 0) + (rotate:QI (match_dup 0) + (const_int 4))) + (set (match_dup 0) + (and:QI (match_dup 0) + (const_int 1)))] + "" + [(set_attr "length" "1,2,2,3") + (set_attr "cc" "set_zn,set_zn,set_zn,clobber")]) + +(define_insn_and_split "*extzv.qihi1" + [(set (match_operand:HI 0 "register_operand" "=*d,*d,*d,r") + (zero_extract:HI (match_operand:QI 1 "register_operand" "0,r,0,r") + (const_int 1) + (match_operand:QI 2 "const_0_to_7_operand" "L,L,P,n")))] + "" + "#" + "" + [(set (match_dup 3) + (zero_extract:QI (match_dup 1) + (const_int 1) + (match_dup 2))) + (set (match_dup 4) + (const_int 0))] + { + operands[3] = simplify_gen_subreg (QImode, operands[0], HImode, 0); + operands[4] = simplify_gen_subreg (QImode, operands[0], HImode, 1); + }) + +(define_insn_and_split "*extzv.qihi2" + [(set (match_operand:HI 0 "register_operand" "=*d,*d,*d,r") + (zero_extend:HI + (zero_extract:QI (match_operand:QI 1 "register_operand" "0,r,0,r") + (const_int 1) + (match_operand:QI 2 "const_0_to_7_operand" "L,L,P,n"))))] + "" + "#" + "" + [(set (match_dup 3) + (zero_extract:QI (match_dup 1) + (const_int 1) + (match_dup 2))) + (set (match_dup 4) + (const_int 0))] + { + operands[3] = simplify_gen_subreg (QImode, operands[0], HImode, 0); + operands[4] = simplify_gen_subreg (QImode, operands[0], HImode, 1); + }) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, AVR]: Fix PR33049 (implement extzv) 2011-06-20 18:39 [Patch, AVR]: Fix PR33049 (implement extzv) Georg-Johann Lay @ 2011-06-21 7:42 ` Denis Chertykov 2011-06-21 10:05 ` Georg-Johann Lay 0 siblings, 1 reply; 8+ messages in thread From: Denis Chertykov @ 2011-06-21 7:42 UTC (permalink / raw) To: Georg-Johann Lay; +Cc: gcc-patches, Anatoly Sokolov, Eric B. Weddington 2011/6/20 Georg-Johann Lay <avr@gjlay.de>: > This is an optimization patch that implements extzv for 1-bit extracts. > +(define_insn_and_split "*extzv" > + [(set (match_operand:QI 0 "register_operand" "=*d,*d,*d,r") > + (zero_extract:QI (match_operand:QI 1 "register_operand" "0,r,0,r") > + (const_int 1) > + (match_operand:QI 2 "const_0_to_7_operand" "L,L,P,n")))] > + "" > + "@ > + andi %0,1 > + mov %0,%1\;andi %0,1 > + lsr %0\;andi %0,1 > + bst %1,%2\;clr %0\;bld %0,0" Why you have a second constraint alternative (*d,r,L) ? IMHO it's unnecessary. > + && REGNO (operands[0]) >= 16" It's not a good practice to refer to register as number. Denis. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, AVR]: Fix PR33049 (implement extzv) 2011-06-21 7:42 ` Denis Chertykov @ 2011-06-21 10:05 ` Georg-Johann Lay 2011-06-21 12:21 ` Denis Chertykov 0 siblings, 1 reply; 8+ messages in thread From: Georg-Johann Lay @ 2011-06-21 10:05 UTC (permalink / raw) To: Denis Chertykov; +Cc: gcc-patches, Anatoly Sokolov, Eric B. Weddington [-- Attachment #1: Type: text/plain, Size: 2143 bytes --] Denis Chertykov schrieb: > 2011/6/20 Georg-Johann Lay <avr@gjlay.de>: >> This is an optimization patch that implements extzv for 1-bit extracts. > > >> +(define_insn_and_split "*extzv" >> + [(set (match_operand:QI 0 "register_operand" "=*d,*d,*d,r") >> + (zero_extract:QI (match_operand:QI 1 "register_operand" "0,r,0,r") >> + (const_int 1) >> + (match_operand:QI 2 "const_0_to_7_operand" "L,L,P,n")))] >> + "" >> + "@ >> + andi %0,1 >> + mov %0,%1\;andi %0,1 >> + lsr %0\;andi %0,1 >> + bst %1,%2\;clr %0\;bld %0,0" > > Why you have a second constraint alternative (*d,r,L) ? > IMHO it's unnecessary. The second alternative takes 2 instructions/ticks whereas the last alternative -- which would be taken without alternative 2 -- takes 3 instructions/ticks. As because of '*' there is no preference on register class, allocator just sees "r,r,n" and if the output register happens to end up in 'd' shorter sequences are possible in some cases. But there is no pressure being put to get such an alternative; in particular, allocator does not see "d,0,L" and does not emit a move to reload input. >> + && REGNO (operands[0]) >= 16" > > It's not a good practice to refer to register as number. > > Denis. Ok. d_register_operand or satisfies_constraint_d would do. But I preferred to introduce a new constraint for 4 and avoid splitting *extzv altogether to keep it simple. Complexity of splitting is not a real advantage over writing things down directly. As avr is running out of constraint letters, I started 3-letter constraints 'Y**' in the style of bfin. Would be nice if 'K' was not already occupied or could be deprecated (is'n not really useful in inline asm)... The split patterns just need one alternative, so there is just this one 'r,r,n' alternative now. Again, tested without regression. Johann PR target/33049 * config/avr/avr.md (extzv): New expander. (*extzv): New insn. (*extzv.qihi1, *extzv.qihi2): New insn-and-split. * config/avr/constraints.md (Y04): New constraint. * doc/md.texi (Machine Constraints): Document it. [-- Attachment #2: pr33049.diff --] [-- Type: text/x-patch, Size: 3496 bytes --] Index: doc/md.texi =================================================================== --- doc/md.texi (revision 175201) +++ doc/md.texi (working copy) @@ -1773,6 +1773,9 @@ Integer constant in the range @minus{}6 @item Q A memory address based on Y or Z pointer with displacement. + +@item Y04 +Constant integer 4 @end table @item Hewlett-Packard PA-RISC---@file{config/pa/pa.h} Index: config/avr/constraints.md =================================================================== --- config/avr/constraints.md (revision 175201) +++ config/avr/constraints.md (working copy) @@ -107,3 +107,8 @@ (define_memory_constraint "Q" "A memory address based on Y or Z pointer with displacement." (and (match_code "mem") (match_test "extra_constraint_Q (op)"))) + +(define_constraint "Y04" + "Constant integer 4." + (and (match_code "const_int") + (match_test "ival == 4"))) Index: config/avr/avr.md =================================================================== --- config/avr/avr.md (revision 175201) +++ config/avr/avr.md (working copy) @@ -3540,3 +3540,65 @@ (define_insn_and_split "*ior<mode>qi.byt int byteno = INTVAL(operands[2]) / BITS_PER_UNIT; operands[4] = simplify_gen_subreg (QImode, operands[0], <MODE>mode, byteno); }) + +(define_expand "extzv" + [(set (match_operand:QI 0 "register_operand" "") + (zero_extract:QI (match_operand:QI 1 "register_operand" "") + (match_operand:QI 2 "const1_operand" "") + (match_operand:QI 3 "const_0_to_7_operand" "")))] + "" + "") + +(define_insn "*extzv" + [(set (match_operand:QI 0 "register_operand" "=*d,*d,*d,*d,r") + (zero_extract:QI (match_operand:QI 1 "register_operand" "0,r,0,0,r") + (const_int 1) + (match_operand:QI 2 "const_0_to_7_operand" "L,L,P,Y04,n")))] + "" + "@ + andi %0,1 + mov %0,%1\;andi %0,1 + lsr %0\;andi %0,1 + swap %0\;andi %0,1 + bst %1,%2\;clr %0\;bld %0,0" + [(set_attr "length" "1,2,2,2,3") + (set_attr "cc" "set_zn,set_zn,set_zn,set_zn,clobber")]) + +(define_insn_and_split "*extzv.qihi1" + [(set (match_operand:HI 0 "register_operand" "=r") + (zero_extract:HI (match_operand:QI 1 "register_operand" "r") + (const_int 1) + (match_operand:QI 2 "const_0_to_7_operand" "n")))] + "" + "#" + "" + [(set (match_dup 3) + (zero_extract:QI (match_dup 1) + (const_int 1) + (match_dup 2))) + (set (match_dup 4) + (const_int 0))] + { + operands[3] = simplify_gen_subreg (QImode, operands[0], HImode, 0); + operands[4] = simplify_gen_subreg (QImode, operands[0], HImode, 1); + }) + +(define_insn_and_split "*extzv.qihi2" + [(set (match_operand:HI 0 "register_operand" "=r") + (zero_extend:HI + (zero_extract:QI (match_operand:QI 1 "register_operand" "r") + (const_int 1) + (match_operand:QI 2 "const_0_to_7_operand" "n"))))] + "" + "#" + "" + [(set (match_dup 3) + (zero_extract:QI (match_dup 1) + (const_int 1) + (match_dup 2))) + (set (match_dup 4) + (const_int 0))] + { + operands[3] = simplify_gen_subreg (QImode, operands[0], HImode, 0); + operands[4] = simplify_gen_subreg (QImode, operands[0], HImode, 1); + }) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, AVR]: Fix PR33049 (implement extzv) 2011-06-21 10:05 ` Georg-Johann Lay @ 2011-06-21 12:21 ` Denis Chertykov 2011-06-21 12:39 ` Georg-Johann Lay 0 siblings, 1 reply; 8+ messages in thread From: Denis Chertykov @ 2011-06-21 12:21 UTC (permalink / raw) To: Georg-Johann Lay; +Cc: gcc-patches, Anatoly Sokolov, Eric B. Weddington 2011/6/21 Georg-Johann Lay <avr@gjlay.de>: > Denis Chertykov schrieb: >> 2011/6/20 Georg-Johann Lay <avr@gjlay.de>: >>> This is an optimization patch that implements extzv for 1-bit extracts. >> >> >>> +(define_insn_and_split "*extzv" >>> + [(set (match_operand:QI 0 "register_operand" "=*d,*d,*d,r") >>> + (zero_extract:QI (match_operand:QI 1 "register_operand" "0,r,0,r") >>> + (const_int 1) >>> + (match_operand:QI 2 "const_0_to_7_operand" "L,L,P,n")))] >>> + "" >>> + "@ >>> + andi %0,1 >>> + mov %0,%1\;andi %0,1 >>> + lsr %0\;andi %0,1 >>> + bst %1,%2\;clr %0\;bld %0,0" >> >> Why you have a second constraint alternative (*d,r,L) ? >> IMHO it's unnecessary. > > The second alternative takes 2 instructions/ticks whereas the last > alternative -- which would be taken without alternative 2 -- takes 3 > instructions/ticks. As because of '*' there is no preference on > register class, allocator just sees "r,r,n" and if the output register > happens to end up in 'd' shorter sequences are possible in some cases. > But there is no pressure being put to get such an alternative; in > particular, allocator does not see "d,0,L" and does not emit a move to > reload input. Ok. > >>> + && REGNO (operands[0]) >= 16" >> >> It's not a good practice to refer to register as number. >> >> Denis. > > Ok. d_register_operand or satisfies_constraint_d would do. But I > preferred to introduce a new constraint for 4 and avoid splitting > *extzv altogether to keep it simple. Complexity of splitting is not a > real advantage over writing things down directly. > > As avr is running out of constraint letters, I started 3-letter > constraints 'Y**' in the style of bfin. Would be nice if 'K' was not > already occupied or could be deprecated (is'n not really useful in > inline asm)... Better to choose starting letter different to pointer register names X,Y,Z. May be better to use underscore '_' ? Denis. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, AVR]: Fix PR33049 (implement extzv) 2011-06-21 12:21 ` Denis Chertykov @ 2011-06-21 12:39 ` Georg-Johann Lay 2011-06-21 13:26 ` Denis Chertykov 0 siblings, 1 reply; 8+ messages in thread From: Georg-Johann Lay @ 2011-06-21 12:39 UTC (permalink / raw) To: Denis Chertykov; +Cc: gcc-patches, Anatoly Sokolov, Eric B. Weddington Denis Chertykov schrieb: > 2011/6/21 Georg-Johann Lay: >> Ok. d_register_operand or satisfies_constraint_d would do. But I >> preferred to introduce a new constraint for 4 and avoid splitting >> *extzv altogether to keep it simple. Complexity of splitting is not a >> real advantage over writing things down directly. >> >> As avr is running out of constraint letters, I started 3-letter >> constraints 'Y**' in the style of bfin. Would be nice if 'K' was not >> already occupied or could be deprecated (is'n not really useful in >> inline asm)... > > Better to choose starting letter different to pointer register names X,Y,Z. > May be better to use underscore '_' ? > > Denis. My preference would be to change 'K' to 'K02' and thus deprecate 'K'. Other letters are 'A', 'B', 'C', D', 'R', 'S', 'T', 'U', 'W', 'Z'. I do not like '_' as constraint. 'R' might be needed in the future for more register constraints (r24, r24-r25, r22-25, ...) and 'R' is a nice prefix. 'C' looks appealing for constant. Johann ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, AVR]: Fix PR33049 (implement extzv) 2011-06-21 12:39 ` Georg-Johann Lay @ 2011-06-21 13:26 ` Denis Chertykov 2011-06-21 16:47 ` Georg-Johann Lay 0 siblings, 1 reply; 8+ messages in thread From: Denis Chertykov @ 2011-06-21 13:26 UTC (permalink / raw) To: Georg-Johann Lay; +Cc: gcc-patches, Anatoly Sokolov, Eric B. Weddington 2011/6/21 Georg-Johann Lay <avr@gjlay.de>: > Denis Chertykov schrieb: >> 2011/6/21 Georg-Johann Lay: > >>> Ok. d_register_operand or satisfies_constraint_d would do. But I >>> preferred to introduce a new constraint for 4 and avoid splitting >>> *extzv altogether to keep it simple. Complexity of splitting is not a >>> real advantage over writing things down directly. >>> >>> As avr is running out of constraint letters, I started 3-letter >>> constraints 'Y**' in the style of bfin. Would be nice if 'K' was not >>> already occupied or could be deprecated (is'n not really useful in >>> inline asm)... >> >> Better to choose starting letter different to pointer register names X,Y,Z. >> May be better to use underscore '_' ? >> >> Denis. > > My preference would be to change 'K' to 'K02' and thus deprecate 'K'. > > Other letters are 'A', 'B', 'C', D', 'R', 'S', 'T', 'U', 'W', 'Z'. > I do not like '_' as constraint. > 'R' might be needed in the future for more register constraints (r24, > r24-r25, r22-25, ...) and 'R' is a nice prefix. > 'C' looks appealing for constant. I'm vote for 'C'. Denis. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, AVR]: Fix PR33049 (implement extzv) 2011-06-21 13:26 ` Denis Chertykov @ 2011-06-21 16:47 ` Georg-Johann Lay 2011-06-21 17:31 ` Denis Chertykov 0 siblings, 1 reply; 8+ messages in thread From: Georg-Johann Lay @ 2011-06-21 16:47 UTC (permalink / raw) To: Denis Chertykov; +Cc: gcc-patches, Anatoly Sokolov, Eric B. Weddington [-- Attachment #1: Type: text/plain, Size: 320 bytes --] Denis Chertykov schrieb: > I'm vote for 'C'. > > Denis. So it's C :-) Johann PR target/33049 * config/avr/avr.md (extzv): New expander. (*extzv): New insn. (*extzv.qihi1, *extzv.qihi2): New insn-and-split. * config/avr/constraints.md (C04): New constraint. * doc/md.texi (Machine Constraints): Document it. [-- Attachment #2: pr33049.diff --] [-- Type: text/x-patch, Size: 3496 bytes --] Index: doc/md.texi =================================================================== --- doc/md.texi (revision 175264) +++ doc/md.texi (working copy) @@ -1773,6 +1773,9 @@ Integer constant in the range @minus{}6 @item Q A memory address based on Y or Z pointer with displacement. + +@item C04 +Constant integer 4 @end table @item Hewlett-Packard PA-RISC---@file{config/pa/pa.h} Index: config/avr/constraints.md =================================================================== --- config/avr/constraints.md (revision 175201) +++ config/avr/constraints.md (working copy) @@ -107,3 +107,8 @@ (define_memory_constraint "Q" "A memory address based on Y or Z pointer with displacement." (and (match_code "mem") (match_test "extra_constraint_Q (op)"))) + +(define_constraint "C04" + "Constant integer 4." + (and (match_code "const_int") + (match_test "ival == 4"))) Index: config/avr/avr.md =================================================================== --- config/avr/avr.md (revision 175264) +++ config/avr/avr.md (working copy) @@ -3539,3 +3539,65 @@ (define_insn_and_split "*ior<mode>qi.byt int byteno = INTVAL(operands[2]) / BITS_PER_UNIT; operands[4] = simplify_gen_subreg (QImode, operands[0], <MODE>mode, byteno); }) + +(define_expand "extzv" + [(set (match_operand:QI 0 "register_operand" "") + (zero_extract:QI (match_operand:QI 1 "register_operand" "") + (match_operand:QI 2 "const1_operand" "") + (match_operand:QI 3 "const_0_to_7_operand" "")))] + "" + "") + +(define_insn "*extzv" + [(set (match_operand:QI 0 "register_operand" "=*d,*d,*d,*d,r") + (zero_extract:QI (match_operand:QI 1 "register_operand" "0,r,0,0,r") + (const_int 1) + (match_operand:QI 2 "const_0_to_7_operand" "L,L,P,C04,n")))] + "" + "@ + andi %0,1 + mov %0,%1\;andi %0,1 + lsr %0\;andi %0,1 + swap %0\;andi %0,1 + bst %1,%2\;clr %0\;bld %0,0" + [(set_attr "length" "1,2,2,2,3") + (set_attr "cc" "set_zn,set_zn,set_zn,set_zn,clobber")]) + +(define_insn_and_split "*extzv.qihi1" + [(set (match_operand:HI 0 "register_operand" "=r") + (zero_extract:HI (match_operand:QI 1 "register_operand" "r") + (const_int 1) + (match_operand:QI 2 "const_0_to_7_operand" "n")))] + "" + "#" + "" + [(set (match_dup 3) + (zero_extract:QI (match_dup 1) + (const_int 1) + (match_dup 2))) + (set (match_dup 4) + (const_int 0))] + { + operands[3] = simplify_gen_subreg (QImode, operands[0], HImode, 0); + operands[4] = simplify_gen_subreg (QImode, operands[0], HImode, 1); + }) + +(define_insn_and_split "*extzv.qihi2" + [(set (match_operand:HI 0 "register_operand" "=r") + (zero_extend:HI + (zero_extract:QI (match_operand:QI 1 "register_operand" "r") + (const_int 1) + (match_operand:QI 2 "const_0_to_7_operand" "n"))))] + "" + "#" + "" + [(set (match_dup 3) + (zero_extract:QI (match_dup 1) + (const_int 1) + (match_dup 2))) + (set (match_dup 4) + (const_int 0))] + { + operands[3] = simplify_gen_subreg (QImode, operands[0], HImode, 0); + operands[4] = simplify_gen_subreg (QImode, operands[0], HImode, 1); + }) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Patch, AVR]: Fix PR33049 (implement extzv) 2011-06-21 16:47 ` Georg-Johann Lay @ 2011-06-21 17:31 ` Denis Chertykov 0 siblings, 0 replies; 8+ messages in thread From: Denis Chertykov @ 2011-06-21 17:31 UTC (permalink / raw) To: Georg-Johann Lay; +Cc: gcc-patches, Anatoly Sokolov, Eric B. Weddington 2011/6/21 Georg-Johann Lay <avr@gjlay.de>: > Denis Chertykov schrieb: > >> I'm vote for 'C'. >> >> Denis. > > So it's C :-) > > Johann > > PR target/33049 > * config/avr/avr.md (extzv): New expander. > (*extzv): New insn. > (*extzv.qihi1, *extzv.qihi2): New insn-and-split. > * config/avr/constraints.md (C04): New constraint. > * doc/md.texi (Machine Constraints): Document it. > Please, commit. Denis. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2011-06-21 17:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-06-20 18:39 [Patch, AVR]: Fix PR33049 (implement extzv) Georg-Johann Lay 2011-06-21 7:42 ` Denis Chertykov 2011-06-21 10:05 ` Georg-Johann Lay 2011-06-21 12:21 ` Denis Chertykov 2011-06-21 12:39 ` Georg-Johann Lay 2011-06-21 13:26 ` Denis Chertykov 2011-06-21 16:47 ` Georg-Johann Lay 2011-06-21 17:31 ` 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).