Re: [PATCH] Don't use subtarget in builtins.c if it has wrong mode (PR middle-end/45567)

[Richard Guenther <richard dot guenther at gmail dot com>]

On Mon, Sep 13, 2010 at 11:05 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> subtarget passed to expand_builtin is either target, or NULL (the latter
> always at -O+, so non-NULL subtarget it isn't tested very well).
> This means it has the same mode as target, thus it is inappropriate to pass
> it to expand_expr as target when expanding one of builtin's arguments unless
> it has the right mode. ?The following patch adds a mode check to
> expand_builtin_unop, in other cases when it is known that the mode will
> never be correct just removes the subtarget argument.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.5?

Ok.

Thanks,
Richard.

> 2010-09-13 ?Jakub Jelinek ?<jakub@redhat.com>
>
> ? ? ? ?PR middle-end/45567
> ? ? ? ?* builtins.c (expand_builtin_interclass_mathfn, expand_builtin_cexpi,
> ? ? ? ?expand_builtin_powi): Remove subtarget argument, pass NULL_RTX instead.
> ? ? ? ?(expand_builtin): Adjust caller.
> ? ? ? ?(expand_builtin_unop): Only use subtarget if it has the right mode.
>
> ? ? ? ?* gcc.dg/pr45567.c: New test.
>
> --- gcc/builtins.c.jj ? 2010-09-09 10:17:41.000000000 +0200
> +++ gcc/builtins.c ? ? ?2010-09-13 11:07:21.000000000 +0200
> @@ -106,9 +106,9 @@ static void expand_errno_check (tree, rt
> ?static rtx expand_builtin_mathfn (tree, rtx, rtx);
> ?static rtx expand_builtin_mathfn_2 (tree, rtx, rtx);
> ?static rtx expand_builtin_mathfn_3 (tree, rtx, rtx);
> -static rtx expand_builtin_interclass_mathfn (tree, rtx, rtx);
> +static rtx expand_builtin_interclass_mathfn (tree, rtx);
> ?static rtx expand_builtin_sincos (tree);
> -static rtx expand_builtin_cexpi (tree, rtx, rtx);
> +static rtx expand_builtin_cexpi (tree, rtx);
> ?static rtx expand_builtin_int_roundingfn (tree, rtx);
> ?static rtx expand_builtin_int_roundingfn_2 (tree, rtx);
> ?static rtx expand_builtin_next_arg (void);
> @@ -2338,11 +2338,10 @@ interclass_mathfn_icode (tree arg, tree
> ? ?isnan, etc).
> ? ?Return 0 if a normal call should be emitted rather than expanding the
> ? ?function in-line. ?EXP is the expression that is a call to the builtin
> - ? function; if convenient, the result should be placed in TARGET.
> - ? SUBTARGET may be used as the target for computing one of EXP's operands. ?*/
> + ? function; if convenient, the result should be placed in TARGET. ?*/
>
> ?static rtx
> -expand_builtin_interclass_mathfn (tree exp, rtx target, rtx subtarget)
> +expand_builtin_interclass_mathfn (tree exp, rtx target)
> ?{
> ? enum insn_code icode = CODE_FOR_nothing;
> ? rtx op0;
> @@ -2375,7 +2374,7 @@ expand_builtin_interclass_mathfn (tree e
> ? ? ? ? side-effects more the once. ?*/
> ? ? ? CALL_EXPR_ARG (exp, 0) = arg = builtin_save_expr (arg);
>
> - ? ? ?op0 = expand_expr (arg, subtarget, VOIDmode, EXPAND_NORMAL);
> + ? ? ?op0 = expand_expr (arg, NULL_RTX, VOIDmode, EXPAND_NORMAL);
>
> ? ? ? if (mode != GET_MODE (op0))
> ? ? ? ?op0 = convert_to_mode (mode, op0, 0);
> @@ -2442,11 +2441,10 @@ expand_builtin_sincos (tree exp)
>
> ?/* Expand a call to the internal cexpi builtin to the sincos math function.
> ? ?EXP is the expression that is a call to the builtin function; if convenient,
> - ? the result should be placed in TARGET. ?SUBTARGET may be used as the target
> - ? for computing one of EXP's operands. ?*/
> + ? the result should be placed in TARGET. ?*/
>
> ?static rtx
> -expand_builtin_cexpi (tree exp, rtx target, rtx subtarget)
> +expand_builtin_cexpi (tree exp, rtx target)
> ?{
> ? tree fndecl = get_callee_fndecl (exp);
> ? tree arg, type;
> @@ -2469,7 +2467,7 @@ expand_builtin_cexpi (tree exp, rtx targ
> ? ? ? op1 = gen_reg_rtx (mode);
> ? ? ? op2 = gen_reg_rtx (mode);
>
> - ? ? ?op0 = expand_expr (arg, subtarget, VOIDmode, EXPAND_NORMAL);
> + ? ? ?op0 = expand_expr (arg, NULL_RTX, VOIDmode, EXPAND_NORMAL);
>
> ? ? ? /* Compute into op1 and op2. ?*/
> ? ? ? expand_twoval_unop (sincos_optab, op0, op2, op1, 0);
> @@ -3215,7 +3213,7 @@ expand_builtin_pow (tree exp, rtx target
> ? ?function; if convenient, the result should be placed in TARGET. ?*/
>
> ?static rtx
> -expand_builtin_powi (tree exp, rtx target, rtx subtarget)
> +expand_builtin_powi (tree exp, rtx target)
> ?{
> ? tree arg0, arg1;
> ? rtx op0, op1;
> @@ -3244,7 +3242,7 @@ expand_builtin_powi (tree exp, rtx targe
> ? ? ? ? ? ? ?|| (optimize_insn_for_speed_p ()
> ? ? ? ? ? ? ? ? ?&& powi_cost (n) <= POWI_MAX_MULTS)))
> ? ? ? ?{
> - ? ? ? ? op0 = expand_expr (arg0, subtarget, VOIDmode, EXPAND_NORMAL);
> + ? ? ? ? op0 = expand_expr (arg0, NULL_RTX, VOIDmode, EXPAND_NORMAL);
> ? ? ? ? ?op0 = force_reg (mode, op0);
> ? ? ? ? ?return expand_powi (op0, mode, n);
> ? ? ? ?}
> @@ -3258,7 +3256,7 @@ expand_builtin_powi (tree exp, rtx targe
> ? if (target == NULL_RTX)
> ? ? target = gen_reg_rtx (mode);
>
> - ?op0 = expand_expr (arg0, subtarget, mode, EXPAND_NORMAL);
> + ?op0 = expand_expr (arg0, NULL_RTX, mode, EXPAND_NORMAL);
> ? if (GET_MODE (op0) != mode)
> ? ? op0 = convert_to_mode (mode, op0, 0);
> ? op1 = expand_expr (arg1, NULL_RTX, mode2, EXPAND_NORMAL);
> @@ -5000,7 +4998,10 @@ expand_builtin_unop (enum machine_mode t
> ? ? return NULL_RTX;
>
> ? /* Compute the argument. ?*/
> - ?op0 = expand_expr (CALL_EXPR_ARG (exp, 0), subtarget,
> + ?op0 = expand_expr (CALL_EXPR_ARG (exp, 0),
> + ? ? ? ? ? ? ? ? ? ?(subtarget
> + ? ? ? ? ? ? ? ? ? ? && (TYPE_MODE (TREE_TYPE (CALL_EXPR_ARG (exp, 0)))
> + ? ? ? ? ? ? ? ? ? ? ? ? == GET_MODE (subtarget))) ? subtarget : NULL_RTX,
> ? ? ? ? ? ? ? ? ? ? VOIDmode, EXPAND_NORMAL);
> ? /* Compute op, into TARGET if possible.
> ? ? ?Set TARGET to wherever the result comes back. ?*/
> @@ -5835,7 +5836,7 @@ expand_builtin (tree exp, rtx target, rt
> ? ? CASE_FLT_FN (BUILT_IN_FINITE):
> ? ? case BUILT_IN_ISFINITE:
> ? ? case BUILT_IN_ISNORMAL:
> - ? ? ?target = expand_builtin_interclass_mathfn (exp, target, subtarget);
> + ? ? ?target = expand_builtin_interclass_mathfn (exp, target);
> ? ? ? if (target)
> ? ? ? ?return target;
> ? ? ? break;
> @@ -5865,7 +5866,7 @@ expand_builtin (tree exp, rtx target, rt
> ? ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_POWI):
> - ? ? ?target = expand_builtin_powi (exp, target, subtarget);
> + ? ? ?target = expand_builtin_powi (exp, target);
> ? ? ? if (target)
> ? ? ? ?return target;
> ? ? ? break;
> @@ -5887,7 +5888,7 @@ expand_builtin (tree exp, rtx target, rt
> ? ? ? break;
>
> ? ? CASE_FLT_FN (BUILT_IN_CEXPI):
> - ? ? ?target = expand_builtin_cexpi (exp, target, subtarget);
> + ? ? ?target = expand_builtin_cexpi (exp, target);
> ? ? ? gcc_assert (target);
> ? ? ? return target;
>
> --- gcc/testsuite/gcc.dg/pr45567.c.jj ? 2010-09-13 11:12:29.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr45567.c ? ? ?2010-09-13 11:12:13.000000000 +0200
> @@ -0,0 +1,9 @@
> +/* PR middle-end/45567 */
> +/* { dg-do compile } */
> +/* { dg-options "-ftree-ter" } */
> +
> +unsigned
> +foo (char c)
> +{
> + ?return __builtin_popcountl ((unsigned long) c);
> +}
>
> ? ? ? ?Jakub
>