Correct an issue with 8-bit/16-bit stores to `_Atomic' QI and HI data objects producing non-BWX assembly such as:
mb insql $17,$16,$17 ldq_u $1,0($16) mskwl $1,$16,$1 bis $17,$1,$17 stq_u $17,0($16) mb for: void atomic_store16 (_Atomic unsigned short *p, unsigned short v) { *p = v; } where an unprotected RMW sequence does not indeed guarantee atomicity of the access to the data object addressed. This is because Alpha machine description does not provide `atomic_storeMODE' patterns and in their absence the middle end assumes plain stores are atomic for data objects whose size is up to the target's word size, which does not stand for non-BWX Alpha hardware, where only SI and DI mode stores are atomic. Fix the issue by adding `atomic_storeQI' and `atomic_storeHI' patterns, derived from corresponding `atomic_exchangeQI' and `atomic_exchangeHI' ones, but with original data not retrieved from the object addressed. Issue reported by Arnd Bergmann. gcc/ PR target/117759 * config/alpha/alpha-protos.h (alpha_expand_atomic_store_12) (alpha_split_atomic_store_12): New prototypes. * config/alpha/alpha.cc (alpha_expand_atomic_store_12) (alpha_split_atomic_store_12): New functions. * config/alpha/sync.md (atomic_store<mode>): New expander. (atomic_store<mode>_1): New insn and splitter. gcc/testsuite/ PR target/117759 * gcc.target/alpha/atomic-clear-hi.c: New file. * gcc.target/alpha/atomic-clear-qi.c: New file. * gcc.target/alpha/atomic-store-di.c: New file. * gcc.target/alpha/atomic-store-hi.c: New file. * gcc.target/alpha/atomic-store-hi-bwx.c: New file. * gcc.target/alpha/atomic-store-qi.c: New file. * gcc.target/alpha/atomic-store-qi-bwx.c: New file. * gcc.target/alpha/atomic-store-si.c: New file. --- Hi, No regressions in `alpha-linux-gnu' testing across all the frontends and libraries (except for Rust, excluded from the configuration). OK for trunk? Also as a grave bug that has eventually led to the removal of EV4 support from the Linux kernel one hand and a localised self-contained target bug fix on the other OK to backport to release branches even though not a regression? I think this will help handling downstream. Maciej --- gcc/config/alpha/alpha-protos.h | 2 gcc/config/alpha/alpha.cc | 69 +++++++++++++++++++ gcc/config/alpha/sync.md | 28 +++++++ gcc/testsuite/gcc.target/alpha/atomic-clear-hi.c | 28 +++++++ gcc/testsuite/gcc.target/alpha/atomic-clear-qi.c | 28 +++++++ gcc/testsuite/gcc.target/alpha/atomic-store-di.c | 22 ++++++ gcc/testsuite/gcc.target/alpha/atomic-store-hi-bwx.c | 22 ++++++ gcc/testsuite/gcc.target/alpha/atomic-store-hi.c | 32 ++++++++ gcc/testsuite/gcc.target/alpha/atomic-store-qi-bwx.c | 22 ++++++ gcc/testsuite/gcc.target/alpha/atomic-store-qi.c | 32 ++++++++ gcc/testsuite/gcc.target/alpha/atomic-store-si.c | 22 ++++++ 11 files changed, 307 insertions(+) gcc-alpha-atomic-store.diff Index: gcc/gcc/config/alpha/alpha-protos.h =================================================================== --- gcc.orig/gcc/config/alpha/alpha-protos.h +++ gcc/gcc/config/alpha/alpha-protos.h @@ -90,6 +90,8 @@ extern void alpha_split_atomic_op (enum extern void alpha_split_compare_and_swap (rtx op[]); extern void alpha_expand_compare_and_swap_12 (rtx op[]); extern void alpha_split_compare_and_swap_12 (rtx op[]); +extern void alpha_expand_atomic_store_12 (rtx op[]); +extern void alpha_split_atomic_store_12 (rtx op[]); extern void alpha_split_atomic_exchange (rtx op[]); extern void alpha_expand_atomic_exchange_12 (rtx op[]); extern void alpha_split_atomic_exchange_12 (rtx op[]); Index: gcc/gcc/config/alpha/alpha.cc =================================================================== --- gcc.orig/gcc/config/alpha/alpha.cc +++ gcc/gcc/config/alpha/alpha.cc @@ -4634,6 +4634,75 @@ alpha_split_compare_and_swap_12 (rtx ope emit_label (XEXP (label2, 0)); } +/* Expand an atomic store operation. */ + +void +alpha_expand_atomic_store_12 (rtx operands[]) +{ + rtx mem, val, model; + machine_mode mode; + rtx addr, align; + + mem = operands[0]; + val = operands[1]; + model = operands[2]; + mode = GET_MODE (mem); + + /* We forced the address into a register via mem_noofs_operand. */ + addr = XEXP (mem, 0); + gcc_assert (register_operand (addr, DImode)); + + align = expand_simple_binop (Pmode, AND, addr, GEN_INT (-8), + NULL_RTX, 1, OPTAB_DIRECT); + + /* Insert val into the correct byte location within the word. */ + if (val != const0_rtx) + val = emit_insxl (mode, val, addr); + + emit_insn (gen_atomic_store_1 (mode, mem, val, align, model)); +} + +void +alpha_split_atomic_store_12 (rtx operands[]) +{ + rtx orig_mem, addr, val, align, scratch; + rtx label, mem, mask, x; + machine_mode mode; + enum memmodel model; + + orig_mem = operands[0]; + val = operands[1]; + align = operands[2]; + model = (enum memmodel) INTVAL (operands[3]); + scratch = operands[4]; + mode = GET_MODE (orig_mem); + addr = XEXP (orig_mem, 0); + + mem = gen_rtx_MEM (DImode, align); + MEM_VOLATILE_P (mem) = MEM_VOLATILE_P (orig_mem); + if (MEM_ALIAS_SET (orig_mem) == ALIAS_SET_MEMORY_BARRIER) + set_mem_alias_set (mem, ALIAS_SET_MEMORY_BARRIER); + + alpha_pre_atomic_barrier (model); + + label = gen_rtx_LABEL_REF (DImode, gen_label_rtx ()); + emit_label (XEXP (label, 0)); + + emit_insn (gen_load_locked (DImode, scratch, mem)); + + mask = GEN_INT (mode == QImode ? 0xff : 0xffff); + emit_insn (gen_mskxl (scratch, scratch, mask, addr)); + if (val != const0_rtx) + emit_insn (gen_iordi3 (scratch, scratch, val)); + + emit_insn (gen_store_conditional (DImode, scratch, mem, scratch)); + + x = gen_rtx_EQ (DImode, scratch, const0_rtx); + emit_unlikely_jump (x, label); + + alpha_post_atomic_barrier (model); +} + /* Expand an atomic exchange operation. */ void Index: gcc/gcc/config/alpha/sync.md =================================================================== --- gcc.orig/gcc/config/alpha/sync.md +++ gcc/gcc/config/alpha/sync.md @@ -159,6 +159,34 @@ } [(set_attr "type" "multi")]) +(define_expand "atomic_store<mode>" + [(match_operand:I12MODE 0 "mem_noofs_operand") ;; memory + (match_operand:I12MODE 1 "reg_or_0_operand") ;; input + (match_operand:SI 2 "const_int_operand")] ;; model + "!TARGET_BWX" +{ + alpha_expand_atomic_store_12 (operands); + DONE; +}) + +(define_insn_and_split "@atomic_store<mode>_1" + [(set (match_operand:I12MODE 0 "mem_noofs_operand" "=w") ;; memory + (unspec:I12MODE + [(match_operand:DI 1 "reg_or_8bit_operand" "rI") ;; input + (match_operand:DI 2 "register_operand" "r") ;; align + (match_operand:SI 3 "const_int_operand")] ;; model + UNSPEC_XCHG)) + (clobber (match_scratch:DI 4 "=&r"))] + "!TARGET_BWX" + "#" + "epilogue_completed" + [(const_int 0)] +{ + alpha_split_atomic_store_12 (operands); + DONE; +} + [(set_attr "type" "multi")]) + (define_insn_and_split "atomic_exchange<mode>" [(set (match_operand:I48MODE 0 "register_operand" "=&r") ;; output (match_operand:I48MODE 1 "memory_operand" "+m")) ;; memory Index: gcc/gcc/testsuite/gcc.target/alpha/atomic-clear-hi.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.target/alpha/atomic-clear-hi.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mno-bwx" } */ + +typedef _Atomic unsigned int __attribute__ ((mode (HI))) atomic16_t; + +void +atomic_clear16 (atomic16_t *p) +{ + *p = 0; +} + +/* Expect assembly such as: + + mb + bic $16,7,$1 +$L3: + ldq_l $2,0($1) + mskwl $2,$16,$2 + stq_c $2,0($1) + beq $2,$L3 + mb + */ + +/* { dg-final { scan-assembler-times "\\smb\\s" 2 } } */ +/* { dg-final { scan-assembler-times "\\sldq_l\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\smskwl\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\sstq_c\\s" 1 } } */ +/* { dg-final { scan-assembler-not "\\s(?:inswl|ldq_u|stq_u)\\s" } } */ Index: gcc/gcc/testsuite/gcc.target/alpha/atomic-clear-qi.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.target/alpha/atomic-clear-qi.c @@ -0,0 +1,28 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mno-bwx" } */ + +typedef _Atomic unsigned int __attribute__ ((mode (QI))) atomic8_t; + +void +atomic_clear8 (atomic8_t *p) +{ + *p = 0; +} + +/* Expect assembly such as: + + mb + bic $16,7,$1 +$L3: + ldq_l $2,0($1) + mskbl $2,$16,$2 + stq_c $2,0($1) + beq $2,$L3 + mb + */ + +/* { dg-final { scan-assembler-times "\\smb\\s" 2 } } */ +/* { dg-final { scan-assembler-times "\\sldq_l\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\smskbl\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\sstq_c\\s" 1 } } */ +/* { dg-final { scan-assembler-not "\\s(?:insbl|ldq_u|stq_u)\\s" } } */ Index: gcc/gcc/testsuite/gcc.target/alpha/atomic-store-di.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.target/alpha/atomic-store-di.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +typedef unsigned int __attribute__ ((mode (DI))) int64_t; +typedef _Atomic int64_t atomic64_t; + +void +atomic_store64 (atomic64_t *p, int64_t v) +{ + *p = v; +} + +/* Expect assembly such as: + + mb + stq $17,0($16) + mb + */ + +/* { dg-final { scan-assembler-times "\\smb\\s" 2 } } */ +/* { dg-final { scan-assembler-times "\\sstq\\s" 1 } } */ +/* { dg-final { scan-assembler-not "\\s(?:ldq_u|stq_u)\\s" } } */ Index: gcc/gcc/testsuite/gcc.target/alpha/atomic-store-hi-bwx.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.target/alpha/atomic-store-hi-bwx.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mbwx" } */ + +typedef unsigned int __attribute__ ((mode (HI))) int16_t; +typedef _Atomic int16_t atomic16_t; + +void +atomic_store16 (atomic16_t *p, int16_t v) +{ + *p = v; +} + +/* Expect assembly such as: + + mb + stw $17,0($16) + mb + */ + +/* { dg-final { scan-assembler-times "\\smb\\s" 2 } } */ +/* { dg-final { scan-assembler-times "\\sstw\\s" 1 } } */ +/* { dg-final { scan-assembler-not "\\s(?:ldq_u|stq_u)\\s" } } */ Index: gcc/gcc/testsuite/gcc.target/alpha/atomic-store-hi.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.target/alpha/atomic-store-hi.c @@ -0,0 +1,32 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mno-bwx" } */ + +typedef unsigned int __attribute__ ((mode (HI))) int16_t; +typedef _Atomic int16_t atomic16_t; + +void +atomic_store16 (atomic16_t *p, int16_t v) +{ + *p = v; +} + +/* Expect assembly such as: + + mb + bic $16,7,$1 + inswl $17,$16,$17 +$L3: + ldq_l $2,0($1) + mskwl $2,$16,$2 + bis $2,$17,$2 + stq_c $2,0($1) + beq $2,$L3 + mb + */ + +/* { dg-final { scan-assembler-times "\\smb\\s" 2 } } */ +/* { dg-final { scan-assembler-times "\\sinswl\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\sldq_l\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\smskwl\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\sstq_c\\s" 1 } } */ +/* { dg-final { scan-assembler-not "\\s(?:ldq_u|stq_u)\\s" } } */ Index: gcc/gcc/testsuite/gcc.target/alpha/atomic-store-qi-bwx.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.target/alpha/atomic-store-qi-bwx.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mbwx" } */ + +typedef unsigned int __attribute__ ((mode (QI))) int8_t; +typedef _Atomic int8_t atomic8_t; + +void +atomic_store8 (atomic8_t *p, int8_t v) +{ + *p = v; +} + +/* Expect assembly such as: + + mb + stb $17,0($16) + mb + */ + +/* { dg-final { scan-assembler-times "\\smb\\s" 2 } } */ +/* { dg-final { scan-assembler-times "\\sstb\\s" 1 } } */ +/* { dg-final { scan-assembler-not "\\s(?:ldq_u|stq_u)\\s" } } */ Index: gcc/gcc/testsuite/gcc.target/alpha/atomic-store-qi.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.target/alpha/atomic-store-qi.c @@ -0,0 +1,32 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -mno-bwx" } */ + +typedef unsigned int __attribute__ ((mode (QI))) int8_t; +typedef _Atomic int8_t atomic8_t; + +void +atomic_store8 (atomic8_t *p, int8_t v) +{ + *p = v; +} + +/* Expect assembly such as: + + mb + bic $16,7,$1 + insbl $17,$16,$17 +$L3: + ldq_l $2,0($1) + mskbl $2,$16,$2 + bis $2,$17,$2 + stq_c $2,0($1) + beq $2,$L3 + mb + */ + +/* { dg-final { scan-assembler-times "\\smb\\s" 2 } } */ +/* { dg-final { scan-assembler-times "\\sinsbl\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\sldq_l\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\smskbl\\s" 1 } } */ +/* { dg-final { scan-assembler-times "\\sstq_c\\s" 1 } } */ +/* { dg-final { scan-assembler-not "\\s(?:ldq_u|stq_u)\\s" } } */ Index: gcc/gcc/testsuite/gcc.target/alpha/atomic-store-si.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.target/alpha/atomic-store-si.c @@ -0,0 +1,22 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +typedef unsigned int __attribute__ ((mode (SI))) int32_t; +typedef _Atomic int32_t atomic32_t; + +void +atomic_store32 (atomic32_t *p, int32_t v) +{ + *p = v; +} + +/* Expect assembly such as: + + mb + stl $17,0($16) + mb + */ + +/* { dg-final { scan-assembler-times "\\smb\\s" 2 } } */ +/* { dg-final { scan-assembler-times "\\sstl\\s" 1 } } */ +/* { dg-final { scan-assembler-not "\\s(?:ldq_u|stq_u)\\s" } } */