On Tue, 14 Jul 2020, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, we generate a useless movzbl insn before lock cmpxchg.
> The problem is that the builtin for the char/short cases has the arguments
> promoted to int and combine gives up, because the instructions have
> MEM_VOLATILE_P arguments and recog in that case doesn't recognize anything
> when volatile_ok is false, and nothing afterwards optimizes the
> (reg:SI a) = (zero_extend:SI (reg:QI a))
> ... (subreg:QI (reg:SI a) 0) ...
So the above isn't fixable? Because it would probably be the more
generic fix, right?
> The following patch fixes it at expansion time, we already have a function
> that is meant to undo the promotion, so this just adds the very common case
> to that.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2020-07-14 Jakub Jelinek <ja...@redhat.com>
>
> PR target/96176
> * builtins.c: Include gimple-ssa.h, tree-ssa-live.h and
> tree-outof-ssa.h.
> (expand_expr_force_mode): If exp is a SSA_NAME with different mode
> from MODE and get_gimple_for_ssa_name is a cast from MODE, use the
> cast's rhs.
>
> * gcc.target/i386/pr96176.c: New test.
>
> --- gcc/builtins.c.jj 2020-06-22 10:59:15.000000000 +0200
> +++ gcc/builtins.c 2020-07-13 12:37:56.519653940 +0200
> @@ -73,6 +73,9 @@ along with GCC; see the file COPYING3.
> #include "gomp-constants.h"
> #include "omp-general.h"
> #include "tree-dfa.h"
> +#include "gimple-ssa.h"
> +#include "tree-ssa-live.h"
> +#include "tree-outof-ssa.h"
>
> struct target_builtins default_target_builtins;
> #if SWITCHABLE_TARGET
> @@ -6671,6 +6674,21 @@ expand_expr_force_mode (tree exp, machin
> rtx val;
> machine_mode old_mode;
>
> + if (TREE_CODE (exp) == SSA_NAME
> + && TYPE_MODE (TREE_TYPE (exp)) != mode)
> + {
> + /* Undo argument promotion if possible, as combine might not
> + be able to do it later due to MEM_VOLATILE_P uses in the
> + patterns. */
> + gimple *g = get_gimple_for_ssa_name (exp);
> + if (g && gimple_assign_cast_p (g))
> + {
> + tree rhs = gimple_assign_rhs1 (g);
> + if (TYPE_MODE (TREE_TYPE (rhs)) == mode)
Is this really enough to check? What if the cast was truncating?
The gimple_assign_cast_p predicate also allows for FIX_TRUNC_EXPR
and VIEW_CONVERT_EXPR where for the latter the rhs is the
VIEW_CONVERT_EXPR itself.
Richard.
> + exp = rhs;
> + }
> + }
> +
> val = expand_expr (exp, NULL_RTX, mode, EXPAND_NORMAL);
> /* If VAL is promoted to a wider mode, convert it back to MODE. Take care
> of CONST_INTs, where we know the old_mode only from the call argument.
> */
> --- gcc/testsuite/gcc.target/i386/pr96176.c.jj 2020-07-13
> 12:44:15.940149701 +0200
> +++ gcc/testsuite/gcc.target/i386/pr96176.c 2020-07-13 12:45:07.822396980
> +0200
> @@ -0,0 +1,13 @@
> +/* PR target/96176 */
> +/* { dg-do compile { target lp64 } } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler-not "\tmovzbl\t" } } */
> +
> +unsigned char v;
> +
> +void
> +foo (unsigned char *x, unsigned char y, unsigned char z)
> +{
> + __atomic_compare_exchange_n (x, &y, z, 0, __ATOMIC_RELAXED,
> __ATOMIC_RELAXED);
> + v = y;
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
