the __sync expanders were defaulting to issuing normal loads and stores surrounded by barriers when no pattern existed.

If the type is larger than the word size of the architecture, the load or store would expand to 2 (or more) loads/stores and not be atomic.

This patch corrects that. In the case of a store, it now first tries to generate an exchange and throw away the result before completely bailing on the operation.

As a result of not being able to generate a 128 bit load correctly now, x86-64 no longer has 128 bit __sync support.

Bootstraps on x86_64-unknown-linux-gnu with no regressions other than the test suite changes brought about by x86-64 not supporting 128 bit __sync's any more.

Andrew
        * optabs.c (expand_sync_mem_load): Don't expand into a default load if
        the type is larger than a word.
        (expand_sync_mem_store): Return const0_rtx if a store is generated. If
        type is larger than a word try an exchange, then fail.
        * builtins.c (expand_builtin_sync_mem_store): Return a value.
        (expand_builtin): If no store generated, leave a function call.
        * expr.h (expand_sync_mem_store): Prototype returns value.

        * testsuite/lib/target-supports.exp: Disable sync_int_128 support for
        x86-64 since a 128 bit atomic load cannot be generated.
        * testsuite/gcc.dg/memmodel/sync-other-int128.c: Don't xfail any more.


Index: optabs.c
===================================================================
*** optabs.c    (revision 178916)
--- optabs.c    (working copy)
*************** expand_sync_mem_load (rtx target, rtx me
*** 7032,7038 ****
        return ops[0].value;
      }
  
!   /* If there is no load, default to a move with barriers. */
    if (target == const0_rtx)
      target = gen_reg_rtx (mode);
  
--- 7032,7043 ----
        return ops[0].value;
      }
  
!   /* If there is no load pattern, default to a move with barriers. If the size
!      of the object is greater than word size on this target, a default load 
!      will not be atomic, so there is no fall back.  */
!   if (GET_MODE_PRECISION(mode) > BITS_PER_WORD)
!     return NULL_RTX;
! 
    if (target == const0_rtx)
      target = gen_reg_rtx (mode);
  
*************** expand_sync_mem_load (rtx target, rtx me
*** 7048,7060 ****
    return target;
  }
  
! /* This function expands the atomic load operation:
!    return the atomically loaded value in MEM.
! 
     MEMMODEL is the memory model variant to use.
!    TARGET is an option place to stick the return value.  */
  
! void
  expand_sync_mem_store (rtx mem, rtx val, enum memmodel model)
  {
    enum machine_mode mode = GET_MODE (mem);
--- 7053,7064 ----
    return target;
  }
  
! /* This function expands the atomic store operation:
!    Atomically store VAL in MEM.
     MEMMODEL is the memory model variant to use.
!    function returns const0_rtx if a pattern was emitted.  */
  
! rtx
  expand_sync_mem_store (rtx mem, rtx val, enum memmodel model)
  {
    enum machine_mode mode = GET_MODE (mem);
*************** expand_sync_mem_store (rtx mem, rtx val,
*** 7070,7076 ****
        create_input_operand (&ops[1], val, mode);
        create_integer_operand (&ops[2], model);
        if (maybe_expand_insn (icode, 3, ops))
!       return;
      }
  
    /* A store of 0 is the same as __sync_lock_release, try that.  */
--- 7074,7080 ----
        create_input_operand (&ops[1], val, mode);
        create_integer_operand (&ops[2], model);
        if (maybe_expand_insn (icode, 3, ops))
!       return const0_rtx;
      }
  
    /* A store of 0 is the same as __sync_lock_release, try that.  */
*************** expand_sync_mem_store (rtx mem, rtx val,
*** 7086,7095 ****
              /* lock_release is only a release barrier.  */
              if (model == MEMMODEL_SEQ_CST)
                expand_builtin_mem_thread_fence (model);
!             return;
            }
        }
      }
    /* If there is no mem_store, default to a move with barriers */
    if (model == MEMMODEL_SEQ_CST || model == MEMMODEL_RELEASE)
      expand_builtin_mem_thread_fence (model);
--- 7090,7112 ----
              /* lock_release is only a release barrier.  */
              if (model == MEMMODEL_SEQ_CST)
                expand_builtin_mem_thread_fence (model);
!             return const0_rtx;
            }
        }
      }
+ 
+   /* If the size of the object is greater than word size on this target,
+      a default store will not be atomic, Try a mem_exchange and throw away
+      the result.  If that doesn't work, don't do anything.  */
+   if (GET_MODE_PRECISION(mode) > BITS_PER_WORD)
+     {
+       rtx target = expand_sync_mem_exchange (NULL_RTX, mem, val, model);
+       if (target)
+         return const0_rtx;
+       else
+         return NULL_RTX;
+     }
+ 
    /* If there is no mem_store, default to a move with barriers */
    if (model == MEMMODEL_SEQ_CST || model == MEMMODEL_RELEASE)
      expand_builtin_mem_thread_fence (model);
*************** expand_sync_mem_store (rtx mem, rtx val,
*** 7099,7104 ****
--- 7116,7123 ----
    /* For SEQ_CST, also emit a barrier after the load.  */
    if (model == MEMMODEL_SEQ_CST)
      expand_builtin_mem_thread_fence (model);
+ 
+   return const0_rtx;
  }
  
  
Index: builtins.c
===================================================================
*** builtins.c  (revision 178916)
--- builtins.c  (working copy)
*************** expand_builtin_sync_mem_load (enum machi
*** 5340,5346 ****
     EXP is the CALL_EXPR.
     TARGET is an optional place for us to store the results.  */
  
! static void
  expand_builtin_sync_mem_store (enum machine_mode mode, tree exp)
  {
    rtx mem, val;
--- 5340,5346 ----
     EXP is the CALL_EXPR.
     TARGET is an optional place for us to store the results.  */
  
! static rtx
  expand_builtin_sync_mem_store (enum machine_mode mode, tree exp)
  {
    rtx mem, val;
*************** expand_builtin_sync_mem_store (enum mach
*** 5352,5365 ****
        && model != MEMMODEL_RELEASE)
      {
        error ("invalid memory model for %<__sync_mem_store%>");
!       return;
      }
  
    /* Expand the operands.  */
    mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
    val = expand_expr_force_mode (CALL_EXPR_ARG (exp, 1), mode);
  
!   expand_sync_mem_store (mem, val, model);
  }
  
  /* Expand the __sync_mem_fetch_XXX intrinsic:
--- 5352,5365 ----
        && model != MEMMODEL_RELEASE)
      {
        error ("invalid memory model for %<__sync_mem_store%>");
!       return NULL_RTX;
      }
  
    /* Expand the operands.  */
    mem = get_builtin_sync_mem (CALL_EXPR_ARG (exp, 0), mode);
    val = expand_expr_force_mode (CALL_EXPR_ARG (exp, 1), mode);
  
!   return expand_sync_mem_store (mem, val, model);
  }
  
  /* Expand the __sync_mem_fetch_XXX intrinsic:
*************** expand_builtin (tree exp, rtx target, rt
*** 6289,6296 ****
      case BUILT_IN_SYNC_MEM_STORE_8:
      case BUILT_IN_SYNC_MEM_STORE_16:
        mode = get_builtin_sync_mode (fcode - BUILT_IN_SYNC_MEM_STORE_1);
!       expand_builtin_sync_mem_store (mode, exp);
!       return const0_rtx;
  
      case BUILT_IN_SYNC_MEM_ADD_FETCH_1:
      case BUILT_IN_SYNC_MEM_ADD_FETCH_2:
--- 6289,6298 ----
      case BUILT_IN_SYNC_MEM_STORE_8:
      case BUILT_IN_SYNC_MEM_STORE_16:
        mode = get_builtin_sync_mode (fcode - BUILT_IN_SYNC_MEM_STORE_1);
!       target = expand_builtin_sync_mem_store (mode, exp);
!       if (target)
!       return const0_rtx;
!       break;
  
      case BUILT_IN_SYNC_MEM_ADD_FETCH_1:
      case BUILT_IN_SYNC_MEM_ADD_FETCH_2:
Index: testsuite/lib/target-supports.exp
===================================================================
*** testsuite/lib/target-supports.exp   (revision 178916)
--- testsuite/lib/target-supports.exp   (working copy)
*************** proc check_effective_target_sync_int_128
*** 3421,3429 ****
          verbose "check_effective_target_sync_int_128: using cached result" 2
      } else {
          set et_sync_int_128_saved 0
-         if { [istarget x86_64-*-*] } {
-            set et_sync_int_128_saved 1
-         }
      }
  
      verbose "check_effective_target_sync_int_128: returning 
$et_sync_int_128_saved" 2
--- 3421,3426 ----
Index: testsuite/gcc.dg/memmodel/sync-other-int128.c
===================================================================
*** testsuite/gcc.dg/memmodel/sync-other-int128.c       (revision 178916)
--- testsuite/gcc.dg/memmodel/sync-other-int128.c       (working copy)
***************
*** 1,10 ****
  /* { dg-do link } */
- 
  /* { dg-require-effective-target sync_int_128 } */
  /* { dg-options "-mcx16" { target { x86_64-*-* } } } */
! 
! /* { dg-final { memmodel-gdb-test { xfail *-*-* } } } */
! 
  
  #include <stdio.h>
  #include "memmodel.h"
--- 1,7 ----
  /* { dg-do link } */
  /* { dg-require-effective-target sync_int_128 } */
  /* { dg-options "-mcx16" { target { x86_64-*-* } } } */
! /* { dg-final { memmodel-gdb-test } } */
  
  #include <stdio.h>
  #include "memmodel.h"
Index: expr.h
===================================================================
*** expr.h      (revision 178916)
--- expr.h      (working copy)
*************** rtx expand_sync_mem_exchange (rtx, rtx, 
*** 221,227 ****
  rtx expand_sync_mem_compare_exchange (rtx, rtx, rtx, rtx, enum memmodel, 
                                      enum memmodel);
  rtx expand_sync_mem_load (rtx, rtx, enum memmodel);
! void expand_sync_mem_store (rtx, rtx, enum memmodel);
  rtx expand_sync_mem_fetch_op (rtx, rtx, rtx, enum rtx_code, enum memmodel, 
                              bool);
  void expand_sync_mem_thread_fence (enum memmodel);
--- 221,227 ----
  rtx expand_sync_mem_compare_exchange (rtx, rtx, rtx, rtx, enum memmodel, 
                                      enum memmodel);
  rtx expand_sync_mem_load (rtx, rtx, enum memmodel);
! rtx expand_sync_mem_store (rtx, rtx, enum memmodel);
  rtx expand_sync_mem_fetch_op (rtx, rtx, rtx, enum rtx_code, enum memmodel, 
                              bool);
  void expand_sync_mem_thread_fence (enum memmodel);

Reply via email to