| Message ID | 4DB7E392.7060001@gjlay.de |
|---|---|
| State | New |
| Headers | show |
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. >
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
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
2011/5/28 Georg-Johann Lay <avr@gjlay.de>: > 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.
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 +;; operation on I/O is atomic. +(define_insn "*insv.io" + [(set (zero_extract:QI (mem:QI (match_operand 0 "low_io_address_operand" "n,n,n")) + (const_int 1) + (match_operand:QI 1 "const_0_to_7_operand" "n,n,n")) + (match_operand:QI 2 "nonmemory_operand" "L,P,r"))] + "" + "@ + 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")]) + +(define_insn "*insv.not.io" + [(set (zero_extract:QI (mem:QI (match_operand 0 "low_io_address_operand" "n")) + (const_int 1) + (match_operand:QI 1 "const_0_to_7_operand" "n")) + (not:QI (match_operand:QI 2 "register_operand" "r")))] + "" + "sbrs %2,0\;sbi %m0-0x20,%1\;sbrc %2,0\;cbi %m0-0x20,%1" + [(set_attr "length" "4") + (set_attr "cc" "none")]) + +;; The insv expander. +;; We only support 1-bit inserts +(define_expand "insv" + [(set (zero_extract:QI (match_operand:QI 0 "register_operand" "") + (match_operand:QI 1 "const1_operand" "") ; width + (match_operand:QI 2 "const_0_to_7_operand" "")) ; pos + (match_operand:QI 3 "nonmemory_operand" ""))] + "optimize" + "") + +;; Insert bit $2.0 into $0.$1 +(define_insn "*insv.reg" + [(set (zero_extract:QI (match_operand:QI 0 "register_operand" "+r,d,d,l,l") + (const_int 1) + (match_operand:QI 1 "const_0_to_7_operand" "n,n,n,n,n")) + (match_operand:QI 2 "nonmemory_operand" "r,L,P,L,P"))] + "" + "@ + bst %2,0\;bld %0,%1 + andi %0,lo8(~(1<<%1)) + ori %0,lo8(1<<%1) + clt\;bld %0,%1 + set\;bld %0,%1" + [(set_attr "length" "2,1,1,2,2") + (set_attr "cc" "none,set_zn,set_zn,none,none")])