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

[Roger Sayle]

Hi Jakub,

> 2006-10-05  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR target/28924
> 	* builtins.c (expand_builtin_sync_operation): fold_convert 2nd argument
> 	to mode's type if it is promoted.
>
> 	* 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-05 15:08:09.000000000 +0200
> @@ -5488,12 +5488,16 @@ expand_builtin_sync_operation (enum mach
>  			       rtx target, bool ignore)
>  {
>    rtx val, mem;
> +  tree value, type;
>
>    /* Expand the operands.  */
>    mem = get_builtin_sync_mem (TREE_VALUE (arglist), mode);
>
>    arglist = TREE_CHAIN (arglist);
> -  val = expand_expr (TREE_VALUE (arglist), NULL, mode, EXPAND_NORMAL);
> +  value = TREE_VALUE (arglist);
> +  type = lang_hooks.types.type_for_size (GET_MODE_BITSIZE (mode), true);
> +  value = fold_convert (type, value);
> +  val = expand_expr (value, NULL, mode, EXPAND_NORMAL);
>
>    if (ignore)
>      return expand_sync_operation (mem, val, code);

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.


Ok for 4.3 with that change (provided it bootstraps and regression
tests and fixes the new test case).

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.

Thanks for fixing this.

Roger
--