This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Fix char/short __sync_fetch_and_xxx (PR target/28924)
- From: Roger Sayle <roger at eyesopen dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Thu, 5 Oct 2006 21:08:28 -0600 (MDT)
- Subject: Re: [PATCH] Fix char/short __sync_fetch_and_xxx (PR target/28924)
> 2006-10-05 Jakub Jelinek <firstname.lastname@example.org>
> 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.