This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix char/short __sync_fetch_and_xxx (PR target/28924, take 2)


On Thu, Oct 05, 2006 at 09:08:28PM -0600, Roger Sayle wrote:
> I'm not a big fan of using lang_hooks in the middle-end if they can be
> avoided.  Any chance that I could ask you to rewrite this using the RTL
> level function, convert_to_mode?  Something like:
> 
> +	val = convert_to_mode (mode, val, 0);
> 
> which is both shorter and avoids the lang_hook dispatch.

Here is the updated patch;  I found that __sync_*_compare_and_swap
and __sync_lock_test_and_set suffered from exactly the same problem,
so I fixed that too.

> If you know of any major code base that's affected by this bug, such
> as libstdc++, the fix might be suitable for 4.2 if you can demonstrate
> it's a regression.  For example, if glibc now uses the x86 sync builtins
> but previously used __asm__, we could leniently/potentially consider
> this a regression.  Otherwise, it needs to wait for stage 1.

I'm afraid I don't have a proof ;(.  libgomp, libstdc++-v3 and libgfortran
all use itself just __sync_* builtins on int and word-sized variables,
glibc doesn't use __sync_* builtins on most arches (uses __asm) and even
if it did, operates again on int/long/pointers and #pragma omp atomic
isn't expanded through through these expand_builtin_sync*.

2006-10-06  Jakub Jelinek  <jakub@redhat.com>

	PR target/28924
	* builtins.c (expand_builtin_sync_operation,
	expand_builtin_compare_and_swap, expand_builtin_lock_test_and_set):
	Use convert_to_mode to handle promoted arguments.

	* gcc.c-torture/compile/20061005-1.c: New test.

--- gcc/builtins.c.jj	2006-10-05 00:28:41.000000000 +0200
+++ gcc/builtins.c	2006-10-06 11:33:00.000000000 +0200
@@ -5494,6 +5494,8 @@ expand_builtin_sync_operation (enum mach
 
   arglist = TREE_CHAIN (arglist);
   val = expand_expr (TREE_VALUE (arglist), NULL, mode, EXPAND_NORMAL);
+  /* If VAL is promoted to a wider mode, convert it back to MODE.  */
+  val = convert_to_mode (mode, val, 1);
 
   if (ignore)
     return expand_sync_operation (mem, val, code);
@@ -5517,9 +5519,13 @@ expand_builtin_compare_and_swap (enum ma
 
   arglist = TREE_CHAIN (arglist);
   old_val = expand_expr (TREE_VALUE (arglist), NULL, mode, EXPAND_NORMAL);
+  /* If OLD_VAL is promoted to a wider mode, convert it back to MODE.  */
+  old_val = convert_to_mode (mode, old_val, 1);
 
   arglist = TREE_CHAIN (arglist);
   new_val = expand_expr (TREE_VALUE (arglist), NULL, mode, EXPAND_NORMAL);
+  /* If NEW_VAL is promoted to a wider mode, convert it back to MODE.  */
+  new_val = convert_to_mode (mode, new_val, 1);
 
   if (is_bool)
     return expand_bool_compare_and_swap (mem, old_val, new_val, target);
@@ -5544,6 +5550,8 @@ expand_builtin_lock_test_and_set (enum m
 
   arglist = TREE_CHAIN (arglist);
   val = expand_expr (TREE_VALUE (arglist), NULL, mode, EXPAND_NORMAL);
+  /* If VAL is promoted to a wider mode, convert it back to MODE.  */
+  val = convert_to_mode (mode, val, 1);
 
   return expand_sync_lock_test_and_set (mem, val, target);
 }
--- gcc/testsuite/gcc.c-torture/compile/20061005-1.c.jj	2006-10-05 15:09:55.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/compile/20061005-1.c	2006-10-06 11:46:28.000000000 +0200
@@ -0,0 +1,23 @@
+/* PR target/28924 */
+
+char c;
+
+void
+testc (void)
+{
+  (void) __sync_fetch_and_add (&c, -1);
+}
+
+short s;
+
+void
+tests (void)
+{
+  (void) __sync_fetch_and_add (&s, -1);
+}
+
+void
+testc2 (void)
+{
+  (void) __sync_val_compare_and_swap (&c, -1, -3);
+}


	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]