Re: [Patch,AVR]: Solve PR42210
2011/5/28 Georg-Johann Lay : > Georg-Johann Lay wrote: >> >> Georg-Johann Lay wrote: >> >>> Richard Henderson wrote: >>> >>> Why are you adding "optimize" to all these insns? None of them will be matched unless combine is run, which implies optimization. >>> >>> Here is a patch without optimize in the insn conditions. >>> >>> The optimize condition is still present in the insv expander because I >>> do not know what the policy about that is in the avr backend. > > http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02099.html > > Hi, I still have a patch hanging around and don't know what to do with it. > Install, change (and if, how) or just throw away. > Please, commit it. Denis.
Re: [Patch,AVR]: Solve PR42210
Georg-Johann Lay wrote: Georg-Johann Lay wrote: Richard Henderson wrote: Why are you adding "optimize" to all these insns? None of them will be matched unless combine is run, which implies optimization. Here is a patch without optimize in the insn conditions. The optimize condition is still present in the insv expander because I do not know what the policy about that is in the avr backend. http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02099.html Hi, I still have a patch hanging around and don't know what to do with it. Install, change (and if, how) or just throw away. Thanks, Johann PR target/42210 * config/avr/predicates.md (const1_operand, const_0_to_7_operand): New predicates. * config/avr/avr.md ("insv"): New insn expander. ("*movbitqi.1-6.a", "*movbitqi.1-6.b", "*movbitqi.0", "*insv.io", "*insv.not.io", "*insv.reg"): New insns. Patch: http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02099.html As Denis wrote in http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00081.html some pattern are too hard to understand. Presumably that are the patches that open-code some bit insertions. Would the path, be ok without these patterns, namely without "*movbitqi.1-6.a" "*movbitqi.1-6.b" "*movbitqi.0" "*movbitqi.7" A am still of the opinion that avr should make use of insn combine and it's power. If these patterns are too hard to understand I can add some more comments and explanations. The insv-like patterns are plain vanilla "insv" "*insv.io" "*insv.reg" These patterns are likely on machines that come with bitfield representations for SFRs. On targets where just SFR addresses and bit positions are available, bit insertions and extractions are open coded by bunch of or-and-not-mask arithmetic, so it's not unlikely to see them in the insn stream. Moreover, RTL lowering won't use insv on volatile mem, just combiner can do that magic. Without the patch there are places where a long, slow 16-bit(!) shift is used requiring a bulky slow loop. If you sag what patterns are ok an which are not, I will make an according patch. (Without combine bridges other patterns might be dead, because combine doesn't look deep enough). Johann
Re: [Patch,AVR]: Solve PR42210
Georg-Johann Lay schrieb: > Richard Henderson schrieb: > >> Why are you adding "optimize" to all these insns? None of them will >> be matched unless combine is run, which implies optimization. > > Here is a patch without optimize in the insn conditions. > > The optimize condition is still present in the insv expander because I > do not know what the policy about that is in the avr backend. > > Johann > >> r~ >> > > PR target/42210 > > * config/avr/predicates.md (const1_operand, const_0_to_7_operand): > New predicates. > > * config/avr/avr.md ("insv"): New insn expander. > ("*movbitqi.1-6.a", "*movbitqi.1-6.b", "*movbitqi.0", "*insv.io", > "*insv.not.io", "*insv.reg"): New insns. > Patch: http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02099.html As Denis wrote in http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00081.html some pattern are too hard to understand. Presumably that are the patches that open-code some bit insertions. Would the path, be ok without these patterns, namely without "*movbitqi.1-6.a" "*movbitqi.1-6.b" "*movbitqi.0" "*movbitqi.7" A am still of the opinion that avr should make use of insn combine and it's power. If these patterns are too hard to understand I can add some more comments and explanations. The insv-like patterns are plain vanilla "insv" "*insv.io" "*insv.reg" These patterns are likely on machines that come with bitfield representations for SFRs. On targets where just SFR addresses and bit positions are available, bit insertions and extractions are open coded by bunch of or-and-not-mask arithmetic, so it's not unlikely to see them in the insn stream. Moreover, RTL lowering won't use insv on volatile mem, just combiner can do that magic. Without the patch there are places where a long, slow 16-bit(!) shift is used requiring a bulky slow loop. If you sag what patterns are ok an which are not, I will make an according patch. (Without combine bridges other patterns might be dead, because combine doesn't look deep enough). Johann
Ping #1: [Patch,AVR]: Solve PR42210
Georg-Johann Lay schrieb: > Richard Henderson schrieb: > >> Why are you adding "optimize" to all these insns? None of them will >> be matched unless combine is run, which implies optimization. > > Here is a patch without optimize in the insn conditions. > > The optimize condition is still present in the insv expander because I > do not know what the policy about that is in the avr backend. > > Johann > >> r~ >> > > PR target/42210 > > * config/avr/predicates.md (const1_operand, const_0_to_7_operand): > New predicates. > > * config/avr/avr.md ("insv"): New insn expander. > ("*movbitqi.1-6.a", "*movbitqi.1-6.b", "*movbitqi.0", "*insv.io", > "*insv.not.io", "*insv.reg"): New insns. >
Re: [Patch,AVR]: Solve PR42210
Richard Henderson schrieb: > Why are you adding "optimize" to all these insns? None of them will > be matched unless combine is run, which implies optimization. Here is a patch without optimize in the insn conditions. The optimize condition is still present in the insv expander because I do not know what the policy about that is in the avr backend. Johann > > r~ > PR target/42210 * config/avr/predicates.md (const1_operand, const_0_to_7_operand): New predicates. * config/avr/avr.md ("insv"): New insn expander. ("*movbitqi.1-6.a", "*movbitqi.1-6.b", "*movbitqi.0", "*insv.io", "*insv.not.io", "*insv.reg"): New insns. Index: config/avr/predicates.md === --- config/avr/predicates.md (Revision 172902) +++ config/avr/predicates.md (Arbeitskopie) @@ -62,6 +62,17 @@ (define_predicate "const0_operand" (and (match_code "const_int,const_double") (match_test "op == CONST0_RTX (mode)"))) +;; Return 1 if OP is the one constant integer for MODE. +(define_predicate "const1_operand" + (and (match_code "const_int") + (match_test "op == CONST1_RTX (mode)"))) + + +;; Return 1 if OP is constant integer 0..7 for MODE. +(define_predicate "const_0_to_7_operand" + (and (match_code "const_int") + (match_test "IN_RANGE (INTVAL (op), 0, 7)"))) + ;; Returns true if OP is either the constant zero or a register. (define_predicate "reg_or_0_operand" (ior (match_operand 0 "register_operand") Index: config/avr/avr.md === --- config/avr/avr.md (Revision 172902) +++ config/avr/avr.md (Arbeitskopie) @@ -3388,3 +3388,116 @@ (define_insn "fmulsu" clr __zero_reg__" [(set_attr "length" "3") (set_attr "cc" "clobber")]) + + +;; Some combiner patterns dealing with bits. +;; See PR42210 + +;; Move bit $3.$4 into bit $0.$4 +(define_insn "*movbitqi.1-6.a" + [(set (match_operand:QI 0 "register_operand" "=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand""0") +(match_operand:QI 2 "single_zero_operand" "n")) +(and:QI (ashift:QI (match_operand:QI 3 "register_operand" "r") + (match_operand:QI 4 "const_0_to_7_operand" "n")) +(match_operand:QI 5 "single_one_operand" "n"] + "INTVAL(operands[4]) == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode)) + && INTVAL(operands[4]) == exact_log2 (INTVAL(operands[5]) & GET_MODE_MASK (QImode))" + "bst %3,0\;bld %0,%4" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $3.0 into bit $0.$4 +;; Variation of above. Unfortunately, there is no canonicalized representation +;; of moving around bits. So what we see here depends on how user writes down +;; bit manipulations. +(define_insn "*movbitqi.1-6.b" + [(set (match_operand:QI 0 "register_operand""=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(match_operand:QI 2 "single_zero_operand" "n")) +(ashift:QI (and:QI (match_operand:QI 3 "register_operand" "r") + (const_int 1)) + (match_operand:QI 4 "const_0_to_7_operand" "n"] + "INTVAL(operands[4]) == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode))" + "bst %3,0\;bld %0,%4" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $3.0 into bit $0.0. +;; For bit 0, combiner generates slightly different pattern. +(define_insn "*movbitqi.0" + [(set (match_operand:QI 0 "register_operand" "=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(match_operand:QI 2 "single_zero_operand" "n")) +(and:QI (match_operand:QI 3 "register_operand" "r") +(const_int 1] + "0 == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode))" + "bst %3,0\;bld %0,0" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $2.0 into bit $0.7. +;; For bit 7, combiner generates slightly different pattern +(define_insn "*movbitqi.7" + [(set (match_operand:QI 0 "register_operand" "=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(const_int 127)) +(ashift:QI (match_operand:QI 2 "register_operand""r") + (const_int 7] + "" + "bst %2,0\;bld %0,7" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Combiner transforms above four pattern into ZERO_EXTRACT if it sees MEM +;; and input/output match. We provide a special pattern for this, because +;; in contrast to a IN/BST/BLD/OUT sequence we need less registers and the +;; oper
Re: [Patch,AVR]: Solve PR42210
Richard Henderson schrieb: > On 04/26/2011 04:51 AM, Georg-Johann Lay wrote: >> As the SFRs are volatile, insv expander skips >> them and a combine pattern must care of them. Omitting the complicated >> bit-plethora patterns I still see long, slow shift loops for HI. So >> the patch still includes them. > > Why don't we focus on doing the right thing for plain registers first, > and worry about volatile SFRs second? The shift loops occur on registers. The insv now covers registers. Besides that, in many places these operations are actually needed to set SFR bits. I see combine try many insv for HImode; I don't know if it is wise to support them without actually knowing what they are used for. Most of this stuff will actually just use one byte of the values, but that can't be seen at that point. > In particular, combine *should* be doing the right thing for registers, > putting those AND/OR/SHIFT whatsits back into insv/extv form, via > make_compound_operation. Yes it should. Bit combine only looks into thins up to some depth, and intermediate patterns can help combine find the way. In the particular case the combine bridges do no harm because they map to efficient instruction sequences in all cases. >> The snip "INTVAL(operands[...])" occurs more than 50 times in the >> machine description. Wouln't it be good to have an abbreviation like >> #define OPVAL(n) INTVAL(operands[n])) defined in, e.g. avr-protos.h? > > No, what would be better is to define more predicates. Though in the > case of comparing op2 vs op4, there's probably no helping it. Was just some a suggestion. I don't see how predicate would help in switch (INTVAL(operands[n])) or similar places in all the C snipps... The predicates you indicated are already included in the last patch. > Why are you adding "optimize" to all these insns? None of them will > be matched unless combine is run, which implies optimization. Similar pattern like *sbi, *cbi etc use that insn condition. I don't know what the rationale behind that is/was, I just followed that. Johann
Re: [Patch,AVR]: Solve PR42210
On 04/26/2011 04:51 AM, Georg-Johann Lay wrote: > As the SFRs are volatile, insv expander skips > them and a combine pattern must care of them. Omitting the complicated > bit-plethora patterns I still see long, slow shift loops for HI. So > the patch still includes them. Why don't we focus on doing the right thing for plain registers first, and worry about volatile SFRs second? In particular, combine *should* be doing the right thing for registers, putting those AND/OR/SHIFT whatsits back into insv/extv form, via make_compound_operation. > The snip "INTVAL(operands[...])" occurs more than 50 times in the > machine description. Wouln't it be good to have an abbreviation like > #define OPVAL(n) INTVAL(operands[n])) defined in, e.g. avr-protos.h? No, what would be better is to define more predicates. Though in the case of comparing op2 vs op4, there's probably no helping it. >> +(define_insn "*movbitqi_ext_reg" >> + [(set (zero_extract:QI >> + (match_operand:QI 0 "register_operand" "+*d,*d,r,r,r") >> + (const_int 1) >> + (match_operand 1 "const_0_to_7_operand" "")) >> +(match_operand:QI 2 "nonme > > Why do you use "*d" as constraint and not "d"? Premature optimization. But what it means is ignore "d" as a register allocation class and only consider "r". Why are you adding "optimize" to all these insns? None of them will be matched unless combine is run, which implies optimization. r~
Re: [Patch,AVR]: Solve PR42210
Richard Henderson schrieb: > On 04/21/2011 05:31 AM, Georg-Johann Lay wrote: >> +;; Some combiner patterns dealing with bits. >> +;; See PR42210 >> + >> +;; Move bit $3.$4 into bit $0.$4 >> +(define_insn "*movbitqi.1-6.a" > ... >> +(define_insn "*movbitqi.1-6.b" > ... >> +(define_insn "*movbitqi.0" > ... >> +(define_insn "*movbitqi.7" > > This looks to be way more complicated than necessary. Consider implementing > the > extzv and insv named patterns, and using zero_extract properly. Hi, I added some more test cases, see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42210#c2 The case of setting bits in bitfield is easy, but for many AVRs there is no header that defines SFRs via bitfields: there are just defines for SFR address and bit position so that user code often contains bulk of AND/OR/SHIFT/NOT etc. As the SFRs are volatile, insv expander skips them and a combine pattern must care of them. Omitting the complicated bit-plethora patterns I still see long, slow shift loops for HI. So the patch still includes them. > E.g. the following, lightly tested. Looking again at the comment on your last > pattern, it looks like that one would still need to be included. Some lines are longer than 79 chars; I prefered legibility as there are already long lines in avr.md, anyway. The snip "INTVAL(operands[...])" occurs more than 50 times in the machine description. Wouln't it be good to have an abbreviation like #define OPVAL(n) INTVAL(operands[n])) defined in, e.g. avr-protos.h? > +(define_insn "*movbitqi_ext_reg" > + [(set (zero_extract:QI > + (match_operand:QI 0 "register_operand" "+*d,*d,r,r,r") > + (const_int 1) > + (match_operand 1 "const_0_to_7_operand" "")) > + (match_operand:QI 2 "nonme Why do you use "*d" as constraint and not "d"? > r~ Johann PR target/42210 * config/avr/predicates.md (const1_operand, const_0_to_7_operand): New predicates. * config/avr/avr.md ("insv"): New insn expander. ("*movbitqi.1-6.a", "*movbitqi.1-6.b", "*movbitqi.0", "*insv.io", "*insv.not.io", "*insv.reg"): New insns. Index: config/avr/predicates.md === --- config/avr/predicates.md (Revision 172902) +++ config/avr/predicates.md (Arbeitskopie) @@ -62,6 +62,17 @@ (define_predicate "const0_operand" (and (match_code "const_int,const_double") (match_test "op == CONST0_RTX (mode)"))) +;; Return 1 if OP is the one constant integer for MODE. +(define_predicate "const1_operand" + (and (match_code "const_int") + (match_test "op == CONST1_RTX (mode)"))) + + +;; Return 1 if OP is constant integer 0..7 for MODE. +(define_predicate "const_0_to_7_operand" + (and (match_code "const_int") + (match_test "IN_RANGE (INTVAL (op), 0, 7)"))) + ;; Returns true if OP is either the constant zero or a register. (define_predicate "reg_or_0_operand" (ior (match_operand 0 "register_operand") Index: config/avr/avr.md === --- config/avr/avr.md (Revision 172902) +++ config/avr/avr.md (Arbeitskopie) @@ -3388,3 +3388,119 @@ (define_insn "fmulsu" clr __zero_reg__" [(set_attr "length" "3") (set_attr "cc" "clobber")]) + + +;; Some combiner patterns dealing with bits. +;; See PR42210 + +;; Move bit $3.$4 into bit $0.$4 +(define_insn "*movbitqi.1-6.a" + [(set (match_operand:QI 0 "register_operand" "=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand""0") +(match_operand:QI 2 "single_zero_operand" "n")) +(and:QI (ashift:QI (match_operand:QI 3 "register_operand" "r") + (match_operand:QI 4 "const_0_to_7_operand" "n")) +(match_operand:QI 5 "single_one_operand" "n"] + "optimize + && INTVAL(operands[4]) == exact_log2 (INTVAL(operands[5]) & GET_MODE_MASK (QImode)) + && INTVAL(operands[4]) == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode))" + "bst %3,0\;bld %0,%4" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $3.0 into bit $0.$4 +;; Variation of above. Unfortunately, there is no canonicalized representation +;; of moving around bits. So what we see here depends on how user writes down +;; bit manipulations. +(define_insn "*movbitqi.1-6.b" + [(set (match_operand:QI 0 "register_operand""=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(match_operand:QI 2 "single_zero_operand" "n")) +(ashift:QI (and:QI (match_operand:QI 3 "register_operand" "r") + (const_int 1)) + (match_operand:QI 4 "const_0_to_7_operand" "n"] + "optimize + && INTVAL(operands[4]) == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QIm
Re: [Patch,AVR]: Solve PR42210
On 04/21/2011 05:31 AM, Georg-Johann Lay wrote: > +;; Some combiner patterns dealing with bits. > +;; See PR42210 > + > +;; Move bit $3.$4 into bit $0.$4 > +(define_insn "*movbitqi.1-6.a" ... > +(define_insn "*movbitqi.1-6.b" ... > +(define_insn "*movbitqi.0" ... > +(define_insn "*movbitqi.7" This looks to be way more complicated than necessary. Consider implementing the extzv and insv named patterns, and using zero_extract properly. E.g. the following, lightly tested. Looking again at the comment on your last pattern, it looks like that one would still need to be included. r~ diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md index 1ab3033..e557fcd 100644 --- a/gcc/config/avr/avr.md +++ b/gcc/config/avr/avr.md @@ -3388,3 +3388,74 @@ clr __zero_reg__" [(set_attr "length" "3") (set_attr "cc" "clobber")]) + +(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_expand "insv" + [(set (zero_extract:QI + (match_operand:QI 0 "register_operand" "") + (match_operand:QI 1 "const1_operand" "") + (match_operand:QI 2 "const_0_to_7_operand" "")) + (match_operand:QI 3 "nonmemory_operand" ""))] + "" + "") + +(define_insn "*movbitqi_reg_ext" + [(set (match_operand:QI 0 "register_operand" "=r") + (zero_extract:QI + (match_operand:QI 1 "register_operand" "r") + (const_int 1) + (match_operand 2 "const_0_to_7_operand" "")))] + "" + "bst %1,%2\;bld %0,0" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +(define_insn "*movbitqi_ext_ext" + [(set (zero_extract:QI + (match_operand:QI 0 "register_operand" "+r") + (const_int 1) + (match_operand 1 "const_0_to_7_operand" "")) + (zero_extract:QI + (match_operand:QI 2 "register_operand" "r") + (const_int 1) + (match_operand 3 "const_0_to_7_operand" "")))] + "" + "bst %2,%3\;bld %0,%1" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +(define_insn "*movbitqi_ext_reg" + [(set (zero_extract:QI + (match_operand:QI 0 "register_operand" "+*d,*d,r,r,r") + (const_int 1) + (match_operand 1 "const_0_to_7_operand" "")) + (match_operand:QI 2 "nonmemory_operand" "L,P,L,P,r"))] + "" +{ + switch (which_alternative) +{ +case 0: + operands[2] = gen_int_mode (~(1 << INTVAL (operands[1])), QImode); + return "andi %0,%2"; +case 1: + operands[2] = gen_int_mode (1 << INTVAL (operands[1]), QImode); + return "ori %0,%2"; +case 2: + return "clt\;bld %0,%1"; +case 3: + return "set\;bld %0,%1"; +case 4: + return "bst %2,0\;bld %0,%1"; +} + gcc_unreachable (); +} + [(set_attr "length" "1,1,2,2,2") + (set_attr "cc" "set_zn,set_zn,none,none,none")]) diff --git a/gcc/config/avr/predicates.md b/gcc/config/avr/predicates.md index 9a3473b..e0142d1 100755 --- a/gcc/config/avr/predicates.md +++ b/gcc/config/avr/predicates.md @@ -62,6 +62,11 @@ (and (match_code "const_int,const_double") (match_test "op == CONST0_RTX (mode)"))) +;; Return 1 if OP is the one constant for MODE. +(define_predicate "const1_operand" + (and (match_code "const_int") + (match_test "op == CONST1_RTX (mode)"))) + ;; Returns true if OP is either the constant zero or a register. (define_predicate "reg_or_0_operand" (ior (match_operand 0 "register_operand") @@ -106,6 +111,11 @@ (and (match_code "const_int") (match_test "exact_log2(~INTVAL (op) & GET_MODE_MASK (mode)) >= 0"))) +;; Return true if OP is a constant between 0 and 7. +(define_predicate "const_0_to_7_operand" + (and (match_code "const_int") + (match_test "IN_RANGE (INTVAL (op), 0, 7)"))) + ;; (define_predicate "avr_sp_immediate_operand" (and (match_code "const_int")
Re: [Patch,AVR]: Solve PR42210
Georg-Johann Lay schrieb: > This solves some missed optimization that can be seen when moving > around bits. > > There are 4 combiner patterns that operate on regs and one that uses > them as intermediate patterns and works on I/O. Even if just an > intermediate pattern matches it's still an improvement because avoid > of shift. > > Tested on some home-brew example. > > Ok if I see no regressions? The original patch has a thinko. The source bit is always 0. Besides that, the (set (zero_extract is now allowed to handle constant sources, too. The new patch fixes that and passes testsuite. Johann > 2011-04-20 Georg-Johann Lay > > PR target/42210 > > * config/avr/avr.md ("*movbitqi.1-6.a", "*movbitqi.1-6.b", > "*movbitqi.0", "*movbitqi.7", "*movbitqi.io"): New insns. > Index: config/avr/avr.md === --- config/avr/avr.md (Revision 172770) +++ config/avr/avr.md (Arbeitskopie) @@ -3388,3 +3388,84 @@ (define_insn "fmulsu" clr __zero_reg__" [(set_attr "length" "3") (set_attr "cc" "clobber")]) + + +;; Some combiner patterns dealing with bits. +;; See PR42210 + +;; Move bit $3.$4 into bit $0.$4 +(define_insn "*movbitqi.1-6.a" + [(set (match_operand:QI 0 "register_operand""=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(match_operand:QI 2 "single_zero_operand" "n")) +(and:QI (ashift:QI (match_operand:QI 3 "register_operand" "r") + (match_operand:QI 4 "const_int_operand" "n")) +(match_operand:QI 5 "single_one_operand" "n"] + "optimize + && INTVAL(operands[4]) == exact_log2 (INTVAL(operands[5]) & GET_MODE_MASK (QImode)) + && INTVAL(operands[4]) == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode))" + "bst %3,0\;bld %0,%4" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $3.0 into bit $0.$4 +;; Variation of above. Unfortunately, there is no canonicalized representation +;; of moving around bits. So what we see here depends on how user writes down +;; bit manipulations. +(define_insn "*movbitqi.1-6.b" + [(set (match_operand:QI 0 "register_operand""=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(match_operand:QI 2 "single_zero_operand" "n")) +(ashift:QI (and:QI (match_operand:QI 3 "register_operand" "r") + (const_int 1)) + (match_operand:QI 4 "const_int_operand" "n"] + "optimize + && INTVAL(operands[4]) == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode))" + "bst %3,0\;bld %0,%4" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $3.0 into bit $0.0. +;; For bit 0, combiner generates slightly different pattern. +(define_insn "*movbitqi.0" + [(set (match_operand:QI 0 "register_operand" "=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(match_operand:QI 2 "single_zero_operand" "n")) +(and:QI (match_operand:QI 3 "register_operand" "r") +(const_int 1] + "optimize + && 0 == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode))" + "bst %3,0\;bld %0,0" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $2.0 into bit $0.7. +;; For bit 7, combiner generates slightly different pattern +(define_insn "*movbitqi.7" + [(set (match_operand:QI 0 "register_operand" "=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(const_int 127)) +(ashift:QI (match_operand:QI 2 "register_operand""r") + (const_int 7] + "optimize" + "bst %2,0\;bld %0,7" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Combiner transforms above four pattern into ZERO_EXTRACT if it sees MEM +;; and input/output match. We provide a special pattern for this, because +;; in contrast to a IN/BST/BLD/OUT sequence we need less registers and the +;; operation on I/O is atomic. +(define_insn "*movbitqi.io" + [(set (zero_extract:QI (mem:QI (match_operand 0 "low_io_address_operand" "n,n,n")) + (const_int 1) + (match_operand 1 "const_int_operand" "n,n,n")) +(match_operand:QI 2 "nonmemory_operand""L,P,r"))] + "optimize + && IN_RANGE (INTVAL(operands[1]), 0, 7)" + "@ + cbi %m0-0x20,%1 + sbi %m0-0x20,%1 + sbrc %2,0\;sbi %m0-0x20,%1\;sbrs %2,0\;cbi %m0-0x20,%1" + [(set_attr "length" "1,1,4") + (set_attr "cc" "none")])
[Patch,AVR]: Solve PR42210
This solves some missed optimization that can be seen when moving around bits. There are 4 combiner patterns that operate on regs and one that uses them as intermediate patterns and works on I/O. Even if just an intermediate pattern matches it's still an improvement because avoid of shift. Tested on some home-brew example. Ok if I see no regressions? Johann 2011-04-20 Georg-Johann Lay PR target/42210 * config/avr/avr.md ("*movbitqi.1-6.a", "*movbitqi.1-6.b", "*movbitqi.0", "*movbitqi.7", "*movbitqi.io"): New insns. Index: config/avr/avr.md === --- config/avr/avr.md (Revision 172770) +++ config/avr/avr.md (Arbeitskopie) @@ -3388,3 +3388,81 @@ (define_insn "fmulsu" clr __zero_reg__" [(set_attr "length" "3") (set_attr "cc" "clobber")]) + + +;; Some combiner patterns dealing with bits. +;; See PR42210 + +;; Move bit $3.$4 into bit $0.$4 +(define_insn "*movbitqi.1-6.a" + [(set (match_operand:QI 0 "register_operand""=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(match_operand:QI 2 "single_zero_operand" "n")) +(and:QI (ashift:QI (match_operand:QI 3 "register_operand" "r") + (match_operand:QI 4 "const_int_operand" "n")) +(match_operand:QI 5 "single_one_operand" "n"] + "optimize + && INTVAL(operands[4]) == exact_log2 (INTVAL(operands[5]) & GET_MODE_MASK (QImode)) + && INTVAL(operands[4]) == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode))" + "bst %3,%4\;bld %0,%4" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $3.$4 into bit $0.$4 +;; Variation of above. Unfortunately, there is no canonicalized representation +;; of moving around bits. So what we see here depends on how user writes down +;; bit manipulations. +(define_insn "*movbitqi.1-6.b" + [(set (match_operand:QI 0 "register_operand""=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(match_operand:QI 2 "single_zero_operand" "n")) +(ashift:QI (and:QI (match_operand:QI 3 "register_operand" "r") + (const_int 1)) + (match_operand:QI 4 "const_int_operand" "n"] + "optimize + && INTVAL(operands[4]) == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode))" + "bst %3,%4\;bld %0,%4" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $3.0 into bit $0.0. +;; For bit 0, combiner generates slightly different pattern. +(define_insn "*movbitqi.0" + [(set (match_operand:QI 0 "register_operand" "=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(match_operand:QI 2 "single_zero_operand" "n")) +(and:QI (match_operand:QI 3 "register_operand" "r") +(const_int 1] + "optimize + && 0 == exact_log2 (~INTVAL(operands[2]) & GET_MODE_MASK (QImode))" + "bst %3,0\;bld %0,0" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Move bit $2.7 into bit $0.7. +;; For bit 7, combiner generates slightly different pattern +(define_insn "*movbitqi.7" + [(set (match_operand:QI 0 "register_operand" "=r") +(ior:QI (and:QI (match_operand:QI 1 "register_operand" "0") +(const_int 127)) +(ashift:QI (match_operand:QI 2 "register_operand""r") + (const_int 7] + "optimize" + "bst %2,7\;bld %0,7" + [(set_attr "length" "2") + (set_attr "cc" "none")]) + +;; Combiner transforms above four pattern into ZERO_EXTRACT if it sees MEM +;; and input/output match. We provide a special pattern for this, because +;; in contrast to a IN/BST/BLD/OUT sequence we need less registers and the +;; operation on I/O is atomic. +(define_insn "*movbitqi.io" + [(set (zero_extract:QI (mem:QI (match_operand 0 "low_io_address_operand" "n")) + (const_int 1) ;; width + (match_operand 1 "const_int_operand" "n")) ;; pos +(match_operand:QI 2 "register_operand" "r"))] + "optimize + && IN_RANGE (INTVAL(operands[1]), 0, 7)" + "sbrc %2,0\;sbi %m0-0x20,%1\;sbrs %2,0\;cbi %m0-0x20,%1" + [(set_attr "length" "4") + (set_attr "cc" "none")])