| Message ID | 53EA2AD4.1070403@arm.com |
|---|---|
| State | New |
| Headers | show |
On 13/08/14 00:55, Alan Lawrence wrote: > ...patch attached... > > Alan Lawrence wrote: >> [When I wrote that xor was broken on GPRs and this fixes it, I meant >> xor_one_cmpl rather than xor, sorry!] >> >> The pattern for xor_one_cmpl never matched, due to the action of >> combine_simplify_rtx; hence, separate this pattern out from that for >> ORN/BIC. >> >> ORN/BIC have equivalent SIMD-reg variants, so add those for the >> benefit of values in vector registers (e.g. passed as [u]int64x1_t >> parameters). >> >> EON does not have a SIMD-reg variant; however, it seems better to >> split it (to XOR + NOT) than to move both arguments to GPRs, perform >> EON, and move the result back. >> +;; (xor (not a) b) is simplify_rtx-ed down to (not (xor a b)). +;; eon does not operate on SIMD registers so the vector variant must be split. +(define_insn_and_split "*xor_one_cmpl<mode>3" + [(set (match_operand:GPI 0 "register_operand" "=r,w") + (not:GPI (xor:GPI (match_operand:GPI 1 "register_operand" "r,?w") Hi Alan, Is there any specific reason for why you are disparaging slightly this alternative with ‘?’. Your earlier patch removes '!' from subdi3. Thanks, Kugan + (match_operand:GPI 2 "register_operand" "r,w"))))] + "" + "eon\\t%<w>0, %<w>1, %<w>2" ;; For GPR registers (only). + "reload_completed && (which_alternative == 1)" ;; For SIMD registers. + [(set (match_operand:GPI 0 "register_operand" "=w") + (xor:GPI (match_operand:GPI 1 "register_operand" "w") + (match_operand:GPI 2 "register_operand" "w"))) + (set (match_dup 0) (not:GPI (match_dup 0)))]
So my reasoning was that the alternative is likely to be more expensive *on all cores*, as it is split to two instructions, whereas add/sub "could" be more expensive for *some* processors. But yes I can see the argument, by my own logic and James Greenhalgh's, for removing the '?': it still doesn't really say what we want to say, which is that the instruction itself is more expensive, rather than anything to do with moving values into registers if/when reloading. At this point in time we don't have a framework that allows us to say that... --Alan Kugan wrote: > On 13/08/14 00:55, Alan Lawrence wrote: >> ...patch attached... >> >> Alan Lawrence wrote: >>> [When I wrote that xor was broken on GPRs and this fixes it, I meant >>> xor_one_cmpl rather than xor, sorry!] >>> >>> The pattern for xor_one_cmpl never matched, due to the action of >>> combine_simplify_rtx; hence, separate this pattern out from that for >>> ORN/BIC. >>> >>> ORN/BIC have equivalent SIMD-reg variants, so add those for the >>> benefit of values in vector registers (e.g. passed as [u]int64x1_t >>> parameters). >>> >>> EON does not have a SIMD-reg variant; however, it seems better to >>> split it (to XOR + NOT) than to move both arguments to GPRs, perform >>> EON, and move the result back. >>> > > > +;; (xor (not a) b) is simplify_rtx-ed down to (not (xor a b)). > +;; eon does not operate on SIMD registers so the vector variant must be > split. > +(define_insn_and_split "*xor_one_cmpl<mode>3" > + [(set (match_operand:GPI 0 "register_operand" "=r,w") > + (not:GPI (xor:GPI (match_operand:GPI 1 "register_operand" "r,?w") > > Hi Alan, > > Is there any specific reason for why you are disparaging slightly this > alternative with ‘?’. Your earlier patch removes '!' from subdi3. > > Thanks, > Kugan > > > + (match_operand:GPI 2 "register_operand" > "r,w"))))] > + "" > + "eon\\t%<w>0, %<w>1, %<w>2" ;; For GPR registers (only). > + "reload_completed && (which_alternative == 1)" ;; For SIMD registers. > + [(set (match_operand:GPI 0 "register_operand" "=w") > + (xor:GPI (match_operand:GPI 1 "register_operand" "w") > + (match_operand:GPI 2 "register_operand" "w"))) > + (set (match_dup 0) (not:GPI (match_dup 0)))] >
On 12 August 2014 15:55, Alan Lawrence <alan.lawrence@arm.com> wrote: >> gcc/ChangeLog: >> >> * config/aarch64/aarch64.c (<LOGICAL:optab>_one_cmpl<mode>3): >> Reparameterize to... >> (<NLOGICAL:optab>_one_cmpl<mode>3): with extra SIMD-register >> variant. >> (xor_one_cmpl<mode>3): New define_insn_and_split. >> >> * config/aarch64/iterators.md (NLOGICAL): New >> define_code_iterator. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/aarch64/eon_1.c: New test. OK /Marcus
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 8eaf1be3ba6e39ca00a2ae3905e84375b354ccd8..2b9cc29148e699b8b6839b6e1294d0eebcad9001 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -2757,14 +2757,36 @@ [(set_attr "type" "logic_shift_imm")] ) -(define_insn "*<LOGICAL:optab>_one_cmpl<mode>3" - [(set (match_operand:GPI 0 "register_operand" "=r") - (LOGICAL:GPI (not:GPI - (match_operand:GPI 1 "register_operand" "r")) - (match_operand:GPI 2 "register_operand" "r")))] +;; Binary logical operators negating one operand, i.e. (a & !b), (a | !b). + +(define_insn "*<NLOGICAL:optab>_one_cmpl<mode>3" + [(set (match_operand:GPI 0 "register_operand" "=r,w") + (NLOGICAL:GPI (not:GPI (match_operand:GPI 1 "register_operand" "r,w")) + (match_operand:GPI 2 "register_operand" "r,w")))] + "" + "@ + <NLOGICAL:nlogical>\\t%<w>0, %<w>2, %<w>1 + <NLOGICAL:nlogical>\\t%0.<Vbtype>, %2.<Vbtype>, %1.<Vbtype>" + [(set_attr "type" "logic_reg,neon_logic") + (set_attr "simd" "*,yes")] +) + +;; (xor (not a) b) is simplify_rtx-ed down to (not (xor a b)). +;; eon does not operate on SIMD registers so the vector variant must be split. +(define_insn_and_split "*xor_one_cmpl<mode>3" + [(set (match_operand:GPI 0 "register_operand" "=r,w") + (not:GPI (xor:GPI (match_operand:GPI 1 "register_operand" "r,?w") + (match_operand:GPI 2 "register_operand" "r,w"))))] + "" + "eon\\t%<w>0, %<w>1, %<w>2" ;; For GPR registers (only). + "reload_completed && (which_alternative == 1)" ;; For SIMD registers. + [(set (match_operand:GPI 0 "register_operand" "=w") + (xor:GPI (match_operand:GPI 1 "register_operand" "w") + (match_operand:GPI 2 "register_operand" "w"))) + (set (match_dup 0) (not:GPI (match_dup 0)))] "" - "<LOGICAL:nlogical>\\t%<w>0, %<w>2, %<w>1" - [(set_attr "type" "logic_reg")] + [(set_attr "type" "logic_reg,multiple") + (set_attr "simd" "*,yes")] ) (define_insn "*and_one_cmpl<mode>3_compare0" diff --git a/gcc/config/aarch64/iterators.md b/gcc/config/aarch64/iterators.md index b7f1d5709eeda0362117f7de3800b99048352225..da8bea2ea4f9e2cc8abae5375b908a247a7edc2f 100644 --- a/gcc/config/aarch64/iterators.md +++ b/gcc/config/aarch64/iterators.md @@ -668,6 +668,9 @@ ;; Code iterator for logical operations (define_code_iterator LOGICAL [and ior xor]) +;; Code iterator for logical operations whose :nlogical works on SIMD registers. +(define_code_iterator NLOGICAL [and ior]) + ;; Code iterator for sign/zero extension (define_code_iterator ANY_EXTEND [sign_extend zero_extend]) diff --git a/gcc/testsuite/gcc.target/aarch64/eon_1.c b/gcc/testsuite/gcc.target/aarch64/eon_1.c new file mode 100644 index 0000000000000000000000000000000000000000..dcdf3b4d052e034e0475028b238bdff0105d4c44 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/eon_1.c @@ -0,0 +1,39 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +/* { dg-final { scan-assembler-not "\tf?mov\t" } } */ + +typedef long long int64_t; +typedef int64_t int64x1_t __attribute__ ((__vector_size__ (8))); + +/* { dg-final { scan-assembler-times "\\teon\\tx\[0-9\]+, x\[0-9\]+, x\[0-9\]+" 1 } } */ + +int64_t +test_eon (int64_t a, int64_t b) +{ + return a ^ ~b; +} + +/* { dg-final { scan-assembler-times "\\tmvn\\tx\[0-9\]+, x\[0-9\]+" 1 } } */ +int64_t +test_not (int64_t a) +{ + return ~a; +} + +/* There is no eon for SIMD regs; we prefer eor+mvn to mov+mov+eon+mov. */ + +/* { dg-final { scan-assembler-times "\\teor\\tv\[0-9\]+\.8b, v\[0-9\]+\.8b, v\[0-9\]+\.8b" 1 } } */ +/* { dg-final { scan-assembler-times "\\tmvn\\tv\[0-9\]+\.8b, v\[0-9\]+\.8b" 2 } } */ +int64x1_t +test_vec_eon (int64x1_t a, int64x1_t b) +{ + return a ^ ~b; +} + +int64x1_t +test_vec_not (int64x1_t a) +{ + return ~a; +} +