Message ID | 20211112152903.117525-1-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | x86: Require TARGET_HIMODE_MATH for HImode atomic bit expanders | expand |
On Fri, Nov 12, 2021 at 07:29:03AM -0800, H.J. Lu wrote: > Check optab before transforming equivalent, but slighly different cases > to their canonical forms in optimize_atomic_bit_test_and and require > TARGET_HIMODE_MATH in HImode atomic bit expanders. > > gcc/ > > PR target/103205 > * tree-ssa-ccp.c (optimize_atomic_bit_test_and): Check optab > before transforming equivalent, but slighly different cases to > their canonical forms. > * config/i386/sync.md (atomic_bit_test_and_set<mode>): Require > TARGET_HIMODE_MATH for HImode. > (atomic_bit_test_and_complement<mode>): Likewise. > (atomic_bit_test_and_reset<mode>): Likewise. > > gcc/testsuite/ > > PR target/103205 > * gcc.target/i386/pr103205-1a.c: New test. > * gcc.target/i386/pr103205-1b.c: Likewise. > * gcc.target/i386/pr103205-2a.c: Likewise. > * gcc.target/i386/pr103205-2b.c: Likewise. > * gcc.target/i386/pr103205-3.c: Likewise. > * gcc.target/i386/pr103205-4.c: Likewise. Why? When one uses 16-bit atomics, no matter what he does there will be some HImode math (at least the atomic instruction). And the rest can be dealt with. I have following patch queued for testing for this... 2021-11-12 Jakub Jelinek <jakub@redhat.com> PR target/103205 * config/i386/sync.md (atomic_bit_test_and_set<mode>, atomic_bit_test_and_complement<mode>, atomic_bit_test_and_reset<mode>): Use OPTAB_WIDEN instead of OPTAB_DIRECT. * gcc.target/i386/pr103205.c: New test. --- gcc/config/i386/sync.md.jj 2021-10-04 19:53:01.025005548 +0200 +++ gcc/config/i386/sync.md 2021-11-12 15:27:47.387273428 +0100 @@ -726,7 +726,7 @@ (define_expand "atomic_bit_test_and_set< rtx result = convert_modes (<MODE>mode, QImode, tem, 1); if (operands[4] == const0_rtx) result = expand_simple_binop (<MODE>mode, ASHIFT, result, - operands[2], operands[0], 0, OPTAB_DIRECT); + operands[2], operands[0], 0, OPTAB_WIDEN); if (result != operands[0]) emit_move_insn (operands[0], result); DONE; @@ -763,7 +763,7 @@ (define_expand "atomic_bit_test_and_comp rtx result = convert_modes (<MODE>mode, QImode, tem, 1); if (operands[4] == const0_rtx) result = expand_simple_binop (<MODE>mode, ASHIFT, result, - operands[2], operands[0], 0, OPTAB_DIRECT); + operands[2], operands[0], 0, OPTAB_WIDEN); if (result != operands[0]) emit_move_insn (operands[0], result); DONE; @@ -801,7 +801,7 @@ (define_expand "atomic_bit_test_and_rese rtx result = convert_modes (<MODE>mode, QImode, tem, 1); if (operands[4] == const0_rtx) result = expand_simple_binop (<MODE>mode, ASHIFT, result, - operands[2], operands[0], 0, OPTAB_DIRECT); + operands[2], operands[0], 0, OPTAB_WIDEN); if (result != operands[0]) emit_move_insn (operands[0], result); DONE; --- gcc/testsuite/gcc.target/i386/pr103205.c.jj 2021-11-12 15:47:21.218380790 +0100 +++ gcc/testsuite/gcc.target/i386/pr103205.c 2021-11-12 15:46:39.546980182 +0100 @@ -0,0 +1,11 @@ +/* PR target/103205 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -mtune-ctrl=^himode_math" } */ + +unsigned short a; + +unsigned short +foo (void) +{ + return __sync_fetch_and_and (&a, ~1) & 1; +} Jakub
On Fri, Nov 12, 2021 at 7:34 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Fri, Nov 12, 2021 at 07:29:03AM -0800, H.J. Lu wrote: > > Check optab before transforming equivalent, but slighly different cases > > to their canonical forms in optimize_atomic_bit_test_and and require > > TARGET_HIMODE_MATH in HImode atomic bit expanders. > > > > gcc/ > > > > PR target/103205 > > * tree-ssa-ccp.c (optimize_atomic_bit_test_and): Check optab > > before transforming equivalent, but slighly different cases to > > their canonical forms. > > * config/i386/sync.md (atomic_bit_test_and_set<mode>): Require > > TARGET_HIMODE_MATH for HImode. > > (atomic_bit_test_and_complement<mode>): Likewise. > > (atomic_bit_test_and_reset<mode>): Likewise. > > > > gcc/testsuite/ > > > > PR target/103205 > > * gcc.target/i386/pr103205-1a.c: New test. > > * gcc.target/i386/pr103205-1b.c: Likewise. > > * gcc.target/i386/pr103205-2a.c: Likewise. > > * gcc.target/i386/pr103205-2b.c: Likewise. > > * gcc.target/i386/pr103205-3.c: Likewise. > > * gcc.target/i386/pr103205-4.c: Likewise. > > Why? When one uses 16-bit atomics, no matter what he does there will be > some HImode math (at least the atomic instruction). And the rest can be > dealt with. I withdrew my patch. > I have following patch queued for testing for this... > > 2021-11-12 Jakub Jelinek <jakub@redhat.com> > > PR target/103205 > * config/i386/sync.md (atomic_bit_test_and_set<mode>, > atomic_bit_test_and_complement<mode>, > atomic_bit_test_and_reset<mode>): Use OPTAB_WIDEN instead of > OPTAB_DIRECT. > > * gcc.target/i386/pr103205.c: New test. Can you include my tests? Or you can leave out your test and I can check in my tests after your fix has been checked in. Thanks. > --- gcc/config/i386/sync.md.jj 2021-10-04 19:53:01.025005548 +0200 > +++ gcc/config/i386/sync.md 2021-11-12 15:27:47.387273428 +0100 > @@ -726,7 +726,7 @@ (define_expand "atomic_bit_test_and_set< > rtx result = convert_modes (<MODE>mode, QImode, tem, 1); > if (operands[4] == const0_rtx) > result = expand_simple_binop (<MODE>mode, ASHIFT, result, > - operands[2], operands[0], 0, OPTAB_DIRECT); > + operands[2], operands[0], 0, OPTAB_WIDEN); > if (result != operands[0]) > emit_move_insn (operands[0], result); > DONE; > @@ -763,7 +763,7 @@ (define_expand "atomic_bit_test_and_comp > rtx result = convert_modes (<MODE>mode, QImode, tem, 1); > if (operands[4] == const0_rtx) > result = expand_simple_binop (<MODE>mode, ASHIFT, result, > - operands[2], operands[0], 0, OPTAB_DIRECT); > + operands[2], operands[0], 0, OPTAB_WIDEN); > if (result != operands[0]) > emit_move_insn (operands[0], result); > DONE; > @@ -801,7 +801,7 @@ (define_expand "atomic_bit_test_and_rese > rtx result = convert_modes (<MODE>mode, QImode, tem, 1); > if (operands[4] == const0_rtx) > result = expand_simple_binop (<MODE>mode, ASHIFT, result, > - operands[2], operands[0], 0, OPTAB_DIRECT); > + operands[2], operands[0], 0, OPTAB_WIDEN); > if (result != operands[0]) > emit_move_insn (operands[0], result); > DONE; > --- gcc/testsuite/gcc.target/i386/pr103205.c.jj 2021-11-12 15:47:21.218380790 +0100 > +++ gcc/testsuite/gcc.target/i386/pr103205.c 2021-11-12 15:46:39.546980182 +0100 > @@ -0,0 +1,11 @@ > +/* PR target/103205 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mtune-ctrl=^himode_math" } */ > + > +unsigned short a; > + > +unsigned short > +foo (void) > +{ > + return __sync_fetch_and_and (&a, ~1) & 1; > +} > > > Jakub >
On Fri, Nov 12, 2021 at 07:55:26AM -0800, H.J. Lu wrote: > > I have following patch queued for testing for this... > > > > 2021-11-12 Jakub Jelinek <jakub@redhat.com> > > > > PR target/103205 > > * config/i386/sync.md (atomic_bit_test_and_set<mode>, > > atomic_bit_test_and_complement<mode>, > > atomic_bit_test_and_reset<mode>): Use OPTAB_WIDEN instead of > > OPTAB_DIRECT. > > > > * gcc.target/i386/pr103205.c: New test. > > Can you include my tests? Or you can leave out your test and I can check > in my tests after your fix has been checked in. I'd prefer the latter. Jakub
On Fri, Nov 12, 2021 at 04:34:27PM +0100, Jakub Jelinek via Gcc-patches wrote: > Why? When one uses 16-bit atomics, no matter what he does there will be > some HImode math (at least the atomic instruction). And the rest can be > dealt with. > > I have following patch queued for testing for this... Bootstrapped/regtested successfully on x86_64-linux and i686-linux, ok for trunk? > 2021-11-12 Jakub Jelinek <jakub@redhat.com> > > PR target/103205 > * config/i386/sync.md (atomic_bit_test_and_set<mode>, > atomic_bit_test_and_complement<mode>, > atomic_bit_test_and_reset<mode>): Use OPTAB_WIDEN instead of > OPTAB_DIRECT. > > * gcc.target/i386/pr103205.c: New test. > > --- gcc/config/i386/sync.md.jj 2021-10-04 19:53:01.025005548 +0200 > +++ gcc/config/i386/sync.md 2021-11-12 15:27:47.387273428 +0100 > @@ -726,7 +726,7 @@ (define_expand "atomic_bit_test_and_set< > rtx result = convert_modes (<MODE>mode, QImode, tem, 1); > if (operands[4] == const0_rtx) > result = expand_simple_binop (<MODE>mode, ASHIFT, result, > - operands[2], operands[0], 0, OPTAB_DIRECT); > + operands[2], operands[0], 0, OPTAB_WIDEN); > if (result != operands[0]) > emit_move_insn (operands[0], result); > DONE; > @@ -763,7 +763,7 @@ (define_expand "atomic_bit_test_and_comp > rtx result = convert_modes (<MODE>mode, QImode, tem, 1); > if (operands[4] == const0_rtx) > result = expand_simple_binop (<MODE>mode, ASHIFT, result, > - operands[2], operands[0], 0, OPTAB_DIRECT); > + operands[2], operands[0], 0, OPTAB_WIDEN); > if (result != operands[0]) > emit_move_insn (operands[0], result); > DONE; > @@ -801,7 +801,7 @@ (define_expand "atomic_bit_test_and_rese > rtx result = convert_modes (<MODE>mode, QImode, tem, 1); > if (operands[4] == const0_rtx) > result = expand_simple_binop (<MODE>mode, ASHIFT, result, > - operands[2], operands[0], 0, OPTAB_DIRECT); > + operands[2], operands[0], 0, OPTAB_WIDEN); > if (result != operands[0]) > emit_move_insn (operands[0], result); > DONE; > --- gcc/testsuite/gcc.target/i386/pr103205.c.jj 2021-11-12 15:47:21.218380790 +0100 > +++ gcc/testsuite/gcc.target/i386/pr103205.c 2021-11-12 15:46:39.546980182 +0100 > @@ -0,0 +1,11 @@ > +/* PR target/103205 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mtune-ctrl=^himode_math" } */ > + > +unsigned short a; > + > +unsigned short > +foo (void) > +{ > + return __sync_fetch_and_and (&a, ~1) & 1; > +} Jakub
On Mon, Nov 15, 2021 at 9:01 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Fri, Nov 12, 2021 at 04:34:27PM +0100, Jakub Jelinek via Gcc-patches wrote: > > Why? When one uses 16-bit atomics, no matter what he does there will be > > some HImode math (at least the atomic instruction). And the rest can be > > dealt with. > > > > I have following patch queued for testing for this... > > Bootstrapped/regtested successfully on x86_64-linux and i686-linux, > ok for trunk? > > > 2021-11-12 Jakub Jelinek <jakub@redhat.com> > > > > PR target/103205 > > * config/i386/sync.md (atomic_bit_test_and_set<mode>, > > atomic_bit_test_and_complement<mode>, > > atomic_bit_test_and_reset<mode>): Use OPTAB_WIDEN instead of > > OPTAB_DIRECT. > > > > * gcc.target/i386/pr103205.c: New test. OK. Thanks, Uros. > > > > --- gcc/config/i386/sync.md.jj 2021-10-04 19:53:01.025005548 +0200 > > +++ gcc/config/i386/sync.md 2021-11-12 15:27:47.387273428 +0100 > > @@ -726,7 +726,7 @@ (define_expand "atomic_bit_test_and_set< > > rtx result = convert_modes (<MODE>mode, QImode, tem, 1); > > if (operands[4] == const0_rtx) > > result = expand_simple_binop (<MODE>mode, ASHIFT, result, > > - operands[2], operands[0], 0, OPTAB_DIRECT); > > + operands[2], operands[0], 0, OPTAB_WIDEN); > > if (result != operands[0]) > > emit_move_insn (operands[0], result); > > DONE; > > @@ -763,7 +763,7 @@ (define_expand "atomic_bit_test_and_comp > > rtx result = convert_modes (<MODE>mode, QImode, tem, 1); > > if (operands[4] == const0_rtx) > > result = expand_simple_binop (<MODE>mode, ASHIFT, result, > > - operands[2], operands[0], 0, OPTAB_DIRECT); > > + operands[2], operands[0], 0, OPTAB_WIDEN); > > if (result != operands[0]) > > emit_move_insn (operands[0], result); > > DONE; > > @@ -801,7 +801,7 @@ (define_expand "atomic_bit_test_and_rese > > rtx result = convert_modes (<MODE>mode, QImode, tem, 1); > > if (operands[4] == const0_rtx) > > result = expand_simple_binop (<MODE>mode, ASHIFT, result, > > - operands[2], operands[0], 0, OPTAB_DIRECT); > > + operands[2], operands[0], 0, OPTAB_WIDEN); > > if (result != operands[0]) > > emit_move_insn (operands[0], result); > > DONE; > > --- gcc/testsuite/gcc.target/i386/pr103205.c.jj 2021-11-12 15:47:21.218380790 +0100 > > +++ gcc/testsuite/gcc.target/i386/pr103205.c 2021-11-12 15:46:39.546980182 +0100 > > @@ -0,0 +1,11 @@ > > +/* PR target/103205 */ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -mtune-ctrl=^himode_math" } */ > > + > > +unsigned short a; > > + > > +unsigned short > > +foo (void) > > +{ > > + return __sync_fetch_and_and (&a, ~1) & 1; > > +} > > Jakub >
diff --git a/gcc/config/i386/sync.md b/gcc/config/i386/sync.md index 05a835256bb..68c4314c21c 100644 --- a/gcc/config/i386/sync.md +++ b/gcc/config/i386/sync.md @@ -717,7 +717,7 @@ (define_expand "atomic_bit_test_and_set<mode>" (match_operand:SWI248 2 "nonmemory_operand") (match_operand:SI 3 "const_int_operand") ;; model (match_operand:SI 4 "const_int_operand")] - "" + "<MODE>mode != HImode || TARGET_HIMODE_MATH" { emit_insn (gen_atomic_bit_test_and_set<mode>_1 (operands[1], operands[2], operands[3])); @@ -753,7 +753,7 @@ (define_expand "atomic_bit_test_and_complement<mode>" (match_operand:SWI248 2 "nonmemory_operand") (match_operand:SI 3 "const_int_operand") ;; model (match_operand:SI 4 "const_int_operand")] - "" + "<MODE>mode != HImode || TARGET_HIMODE_MATH" { emit_insn (gen_atomic_bit_test_and_complement<mode>_1 (operands[1], operands[2], @@ -792,7 +792,7 @@ (define_expand "atomic_bit_test_and_reset<mode>" (match_operand:SWI248 2 "nonmemory_operand") (match_operand:SI 3 "const_int_operand") ;; model (match_operand:SI 4 "const_int_operand")] - "" + "<MODE>mode != HImode || TARGET_HIMODE_MATH" { emit_insn (gen_atomic_bit_test_and_reset<mode>_1 (operands[1], operands[2], operands[3])); diff --git a/gcc/testsuite/gcc.target/i386/pr103205-1a.c b/gcc/testsuite/gcc.target/i386/pr103205-1a.c new file mode 100644 index 00000000000..3ea74b68059 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103205-1a.c @@ -0,0 +1,27 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mtune-ctrl=himode_math" } */ + +extern short foo; + +int +foo1 (void) +{ + return __sync_fetch_and_and(&foo, ~1) & 1; +} + +int +foo2 (void) +{ + return __sync_fetch_and_or (&foo, 1) & 1; +} + +int +foo3 (void) +{ + return __sync_fetch_and_xor (&foo, 1) & 1; +} + +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*btrw" 1 } } */ +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*btsw" 1 } } */ +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*btcw" 1 } } */ +/* { dg-final { scan-assembler-not "cmpxchgw" } } */ diff --git a/gcc/testsuite/gcc.target/i386/pr103205-1b.c b/gcc/testsuite/gcc.target/i386/pr103205-1b.c new file mode 100644 index 00000000000..4ce24b5011e --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103205-1b.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mtune-ctrl=^himode_math" } */ + +#include "pr103205-1a.c" + +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*cmpxchgw" 3 } } */ +/* { dg-final { scan-assembler-not "btrw" } } */ +/* { dg-final { scan-assembler-not "btsw" } } */ +/* { dg-final { scan-assembler-not "btcw" } } */ diff --git a/gcc/testsuite/gcc.target/i386/pr103205-2a.c b/gcc/testsuite/gcc.target/i386/pr103205-2a.c new file mode 100644 index 00000000000..7eb7122aaaf --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103205-2a.c @@ -0,0 +1,26 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mtune-ctrl=himode_math" } */ + +extern unsigned short foo; +unsigned short +foo1 (void) +{ + return __sync_fetch_and_and(&foo, ~1) & 1; +} + +unsigned short +foo2 (void) +{ + return __sync_fetch_and_or (&foo, 1) & 1; +} + +unsigned short +foo3 (void) +{ + return __sync_fetch_and_xor (&foo, 1) & 1; +} + +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*btrw" 1 } } */ +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*btsw" 1 } } */ +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*btcw" 1 } } */ +/* { dg-final { scan-assembler-not "cmpxchgw" } } */ diff --git a/gcc/testsuite/gcc.target/i386/pr103205-2b.c b/gcc/testsuite/gcc.target/i386/pr103205-2b.c new file mode 100644 index 00000000000..49be2e2f39d --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103205-2b.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mtune-ctrl=^himode_math" } */ + +#include "pr103205-2a.c" + +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*cmpxchgw" 3 } } */ +/* { dg-final { scan-assembler-not "btrw" } } */ +/* { dg-final { scan-assembler-not "btsw" } } */ +/* { dg-final { scan-assembler-not "btcw" } } */ diff --git a/gcc/testsuite/gcc.target/i386/pr103205-3.c b/gcc/testsuite/gcc.target/i386/pr103205-3.c new file mode 100644 index 00000000000..609a674242c --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103205-3.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +char sync_fetch_and_and_short_15_a; +int +__attribute__sync_fetch_and_and_short_15 (void) +{ + return __sync_fetch_and_and(&sync_fetch_and_and_short_15_a, ~1) & 1; +} + +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*cmpxchgb" 1 } } */ diff --git a/gcc/testsuite/gcc.target/i386/pr103205-4.c b/gcc/testsuite/gcc.target/i386/pr103205-4.c new file mode 100644 index 00000000000..825867cd705 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr103205-4.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +unsigned char sync_fetch_and_and_short_15_a; +unsigned char +__attribute__sync_fetch_and_and_short_15 (void) +{ + return __sync_fetch_and_and(&sync_fetch_and_and_short_15_a, ~1) & 1; +} + +/* { dg-final { scan-assembler-times "lock;?\[ \t\]*cmpxchgb" 1 } } */ diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c index 0f79e9f05bd..fec68b5fc73 100644 --- a/gcc/tree-ssa-ccp.c +++ b/gcc/tree-ssa-ccp.c @@ -3366,6 +3366,21 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator *gsip, || !gimple_vdef (call)) return; + switch (fn) + { + case IFN_ATOMIC_BIT_TEST_AND_SET: + optab = atomic_bit_test_and_set_optab; + break; + case IFN_ATOMIC_BIT_TEST_AND_COMPLEMENT: + optab = atomic_bit_test_and_complement_optab; + break; + case IFN_ATOMIC_BIT_TEST_AND_RESET: + optab = atomic_bit_test_and_reset_optab; + break; + default: + return; + } + tree bit = nullptr; mask = gimple_call_arg (call, 1); @@ -3384,6 +3399,10 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator *gsip, if (lhs != use_rhs) return; + if (optab_handler (optab, TYPE_MODE (TREE_TYPE (lhs))) + == CODE_FOR_nothing) + return; + gimple *g; gimple_stmt_iterator gsi; tree var; @@ -3628,21 +3647,6 @@ optimize_atomic_bit_test_and (gimple_stmt_iterator *gsip, } } - switch (fn) - { - case IFN_ATOMIC_BIT_TEST_AND_SET: - optab = atomic_bit_test_and_set_optab; - break; - case IFN_ATOMIC_BIT_TEST_AND_COMPLEMENT: - optab = atomic_bit_test_and_complement_optab; - break; - case IFN_ATOMIC_BIT_TEST_AND_RESET: - optab = atomic_bit_test_and_reset_optab; - break; - default: - return; - } - if (optab_handler (optab, TYPE_MODE (TREE_TYPE (lhs))) == CODE_FOR_nothing) return;