* RFA: Fix reuse of "target" variable in builtins.c
@ 2012-12-23 17:02 Richard Sandiford
2012-12-30 19:24 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Richard Sandiford @ 2012-12-23 17:02 UTC (permalink / raw)
To: gcc-patches
Some of the maths builtins try to expand via an optab and fall back
to an expand_call. The optabs path tends to clobber "target",
so the original target is lost by the time we call expand_call. E.g.:
/* Compute into TARGET.
Set TARGET to wherever the result comes back. */
target = expand_binop (mode, builtin_optab, op0, op1,
target, 0, OPTAB_DIRECT);
/* If we were unable to expand via the builtin, stop the sequence
(without outputting the insns) and call to the library function
with the stabilized argument list. */
if (target == 0)
{
end_sequence ();
return expand_call (exp, target, target == const0_rtx);
}
where the expand_call seems to be trying to use the original call target
(as it should IMO) but actually always uses null.
This patch tries to fix the cases I could see. Tested on x86_64-linux-gnu
and mips64-linux-gnu. OK to install?
Richard
gcc/
* builtins.c (expand_builtin_mathfn, expand_builtin_mathfn_2)
(expand_builtin_mathfn_ternary, expand_builtin_mathfn_3)
(expand_builtin_int_roundingfn_2): Keep the original target around
for the fallback case.
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c 2012-12-23 16:56:30.218846420 +0000
+++ gcc/builtins.c 2012-12-23 16:57:47.849415792 +0000
@@ -2031,7 +2031,7 @@ expand_builtin_mathfn (tree exp, rtx tar
if (optab_handler (builtin_optab, mode) != CODE_FOR_nothing
&& (!errno_set || !optimize_insn_for_size_p ()))
{
- target = gen_reg_rtx (mode);
+ rtx result = gen_reg_rtx (mode);
/* Wrap the computation of the argument in a SAVE_EXPR, as we may
need to expand the argument again. This way, we will not perform
@@ -2042,20 +2042,20 @@ expand_builtin_mathfn (tree exp, rtx tar
start_sequence ();
- /* Compute into TARGET.
- Set TARGET to wherever the result comes back. */
- target = expand_unop (mode, builtin_optab, op0, target, 0);
+ /* Compute into RESULT.
+ Set RESULT to wherever the result comes back. */
+ result = expand_unop (mode, builtin_optab, op0, result, 0);
- if (target != 0)
+ if (result != 0)
{
if (errno_set)
- expand_errno_check (exp, target);
+ expand_errno_check (exp, result);
/* Output the entire sequence. */
insns = get_insns ();
end_sequence ();
emit_insn (insns);
- return target;
+ return result;
}
/* If we were unable to expand via the builtin, stop the sequence
@@ -2078,7 +2078,7 @@ expand_builtin_mathfn (tree exp, rtx tar
expand_builtin_mathfn_2 (tree exp, rtx target, rtx subtarget)
{
optab builtin_optab;
- rtx op0, op1, insns;
+ rtx op0, op1, insns, result;
int op1_type = REAL_TYPE;
tree fndecl = get_callee_fndecl (exp);
tree arg0, arg1;
@@ -2134,7 +2134,7 @@ expand_builtin_mathfn_2 (tree exp, rtx t
if (optab_handler (builtin_optab, mode) == CODE_FOR_nothing)
return NULL_RTX;
- target = gen_reg_rtx (mode);
+ result = gen_reg_rtx (mode);
if (! flag_errno_math || ! HONOR_NANS (mode))
errno_set = false;
@@ -2151,29 +2151,29 @@ expand_builtin_mathfn_2 (tree exp, rtx t
start_sequence ();
- /* Compute into TARGET.
- Set TARGET to wherever the result comes back. */
- target = expand_binop (mode, builtin_optab, op0, op1,
- target, 0, OPTAB_DIRECT);
+ /* Compute into RESULT.
+ Set RESULT to wherever the result comes back. */
+ result = expand_binop (mode, builtin_optab, op0, op1,
+ result, 0, OPTAB_DIRECT);
/* If we were unable to expand via the builtin, stop the sequence
(without outputting the insns) and call to the library function
with the stabilized argument list. */
- if (target == 0)
+ if (result == 0)
{
end_sequence ();
return expand_call (exp, target, target == const0_rtx);
}
if (errno_set)
- expand_errno_check (exp, target);
+ expand_errno_check (exp, result);
/* Output the entire sequence. */
insns = get_insns ();
end_sequence ();
emit_insn (insns);
- return target;
+ return result;
}
/* Expand a call to the builtin trinary math functions (fma).
@@ -2187,7 +2187,7 @@ expand_builtin_mathfn_2 (tree exp, rtx t
expand_builtin_mathfn_ternary (tree exp, rtx target, rtx subtarget)
{
optab builtin_optab;
- rtx op0, op1, op2, insns;
+ rtx op0, op1, op2, insns, result;
tree fndecl = get_callee_fndecl (exp);
tree arg0, arg1, arg2;
enum machine_mode mode;
@@ -2214,7 +2214,7 @@ expand_builtin_mathfn_ternary (tree exp,
if (optab_handler (builtin_optab, mode) == CODE_FOR_nothing)
return NULL_RTX;
- target = gen_reg_rtx (mode);
+ result = gen_reg_rtx (mode);
/* Always stabilize the argument list. */
CALL_EXPR_ARG (exp, 0) = arg0 = builtin_save_expr (arg0);
@@ -2227,15 +2227,15 @@ expand_builtin_mathfn_ternary (tree exp,
start_sequence ();
- /* Compute into TARGET.
- Set TARGET to wherever the result comes back. */
- target = expand_ternary_op (mode, builtin_optab, op0, op1, op2,
- target, 0);
+ /* Compute into RESULT.
+ Set RESULT to wherever the result comes back. */
+ result = expand_ternary_op (mode, builtin_optab, op0, op1, op2,
+ result, 0);
/* If we were unable to expand via the builtin, stop the sequence
(without outputting the insns) and call to the library function
with the stabilized argument list. */
- if (target == 0)
+ if (result == 0)
{
end_sequence ();
return expand_call (exp, target, target == const0_rtx);
@@ -2246,7 +2246,7 @@ expand_builtin_mathfn_ternary (tree exp,
end_sequence ();
emit_insn (insns);
- return target;
+ return result;
}
/* Expand a call to the builtin sin and cos math functions.
@@ -2298,7 +2298,7 @@ expand_builtin_mathfn_3 (tree exp, rtx t
/* Before working hard, check whether the instruction is available. */
if (optab_handler (builtin_optab, mode) != CODE_FOR_nothing)
{
- target = gen_reg_rtx (mode);
+ rtx result = gen_reg_rtx (mode);
/* Wrap the computation of the argument in a SAVE_EXPR, as we may
need to expand the argument again. This way, we will not perform
@@ -2309,37 +2309,35 @@ expand_builtin_mathfn_3 (tree exp, rtx t
start_sequence ();
- /* Compute into TARGET.
- Set TARGET to wherever the result comes back. */
+ /* Compute into RESULT.
+ Set RESULT to wherever the result comes back. */
if (builtin_optab == sincos_optab)
{
- int result;
+ int ok;
switch (DECL_FUNCTION_CODE (fndecl))
{
CASE_FLT_FN (BUILT_IN_SIN):
- result = expand_twoval_unop (builtin_optab, op0, 0, target, 0);
+ ok = expand_twoval_unop (builtin_optab, op0, 0, result, 0);
break;
CASE_FLT_FN (BUILT_IN_COS):
- result = expand_twoval_unop (builtin_optab, op0, target, 0, 0);
+ ok = expand_twoval_unop (builtin_optab, op0, result, 0, 0);
break;
default:
gcc_unreachable ();
}
- gcc_assert (result);
+ gcc_assert (ok);
}
else
- {
- target = expand_unop (mode, builtin_optab, op0, target, 0);
- }
+ result = expand_unop (mode, builtin_optab, op0, result, 0);
- if (target != 0)
+ if (result != 0)
{
/* Output the entire sequence. */
insns = get_insns ();
end_sequence ();
emit_insn (insns);
- return target;
+ return result;
}
/* If we were unable to expand via the builtin, stop the sequence
@@ -2348,9 +2346,7 @@ expand_builtin_mathfn_3 (tree exp, rtx t
end_sequence ();
}
- target = expand_call (exp, target, target == const0_rtx);
-
- return target;
+ return expand_call (exp, target, target == const0_rtx);
}
/* Given an interclass math builtin decl FNDECL and it's argument ARG
@@ -2820,7 +2816,7 @@ expand_builtin_int_roundingfn_2 (tree ex
/* There's no easy way to detect the case we need to set EDOM. */
if (!flag_errno_math)
{
- target = gen_reg_rtx (mode);
+ rtx result = gen_reg_rtx (mode);
/* Wrap the computation of the argument in a SAVE_EXPR, as we may
need to expand the argument again. This way, we will not perform
@@ -2831,13 +2827,13 @@ expand_builtin_int_roundingfn_2 (tree ex
start_sequence ();
- if (expand_sfix_optab (target, op0, builtin_optab))
+ if (expand_sfix_optab (result, op0, builtin_optab))
{
/* Output the entire sequence. */
insns = get_insns ();
end_sequence ();
emit_insn (insns);
- return target;
+ return result;
}
/* If we were unable to expand via the builtin, stop the sequence
@@ -2865,9 +2861,7 @@ expand_builtin_int_roundingfn_2 (tree ex
return convert_to_mode (mode, target, 0);
}
- target = expand_call (exp, target, target == const0_rtx);
-
- return target;
+ return expand_call (exp, target, target == const0_rtx);
}
/* Expand a call to the powi built-in mathematical function. Return NULL_RTX if
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: RFA: Fix reuse of "target" variable in builtins.c
2012-12-23 17:02 RFA: Fix reuse of "target" variable in builtins.c Richard Sandiford
@ 2012-12-30 19:24 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2012-12-30 19:24 UTC (permalink / raw)
To: gcc-patches, rdsandiford
On Sun, Dec 23, 2012 at 6:02 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Some of the maths builtins try to expand via an optab and fall back
> to an expand_call. The optabs path tends to clobber "target",
> so the original target is lost by the time we call expand_call. E.g.:
>
> /* Compute into TARGET.
> Set TARGET to wherever the result comes back. */
> target = expand_binop (mode, builtin_optab, op0, op1,
> target, 0, OPTAB_DIRECT);
>
> /* If we were unable to expand via the builtin, stop the sequence
> (without outputting the insns) and call to the library function
> with the stabilized argument list. */
> if (target == 0)
> {
> end_sequence ();
> return expand_call (exp, target, target == const0_rtx);
> }
>
> where the expand_call seems to be trying to use the original call target
> (as it should IMO) but actually always uses null.
>
> This patch tries to fix the cases I could see. Tested on x86_64-linux-gnu
> and mips64-linux-gnu. OK to install?
Ok.
Thanks,
Richard.
> Richard
>
>
> gcc/
> * builtins.c (expand_builtin_mathfn, expand_builtin_mathfn_2)
> (expand_builtin_mathfn_ternary, expand_builtin_mathfn_3)
> (expand_builtin_int_roundingfn_2): Keep the original target around
> for the fallback case.
>
> Index: gcc/builtins.c
> ===================================================================
> --- gcc/builtins.c 2012-12-23 16:56:30.218846420 +0000
> +++ gcc/builtins.c 2012-12-23 16:57:47.849415792 +0000
> @@ -2031,7 +2031,7 @@ expand_builtin_mathfn (tree exp, rtx tar
> if (optab_handler (builtin_optab, mode) != CODE_FOR_nothing
> && (!errno_set || !optimize_insn_for_size_p ()))
> {
> - target = gen_reg_rtx (mode);
> + rtx result = gen_reg_rtx (mode);
>
> /* Wrap the computation of the argument in a SAVE_EXPR, as we may
> need to expand the argument again. This way, we will not perform
> @@ -2042,20 +2042,20 @@ expand_builtin_mathfn (tree exp, rtx tar
>
> start_sequence ();
>
> - /* Compute into TARGET.
> - Set TARGET to wherever the result comes back. */
> - target = expand_unop (mode, builtin_optab, op0, target, 0);
> + /* Compute into RESULT.
> + Set RESULT to wherever the result comes back. */
> + result = expand_unop (mode, builtin_optab, op0, result, 0);
>
> - if (target != 0)
> + if (result != 0)
> {
> if (errno_set)
> - expand_errno_check (exp, target);
> + expand_errno_check (exp, result);
>
> /* Output the entire sequence. */
> insns = get_insns ();
> end_sequence ();
> emit_insn (insns);
> - return target;
> + return result;
> }
>
> /* If we were unable to expand via the builtin, stop the sequence
> @@ -2078,7 +2078,7 @@ expand_builtin_mathfn (tree exp, rtx tar
> expand_builtin_mathfn_2 (tree exp, rtx target, rtx subtarget)
> {
> optab builtin_optab;
> - rtx op0, op1, insns;
> + rtx op0, op1, insns, result;
> int op1_type = REAL_TYPE;
> tree fndecl = get_callee_fndecl (exp);
> tree arg0, arg1;
> @@ -2134,7 +2134,7 @@ expand_builtin_mathfn_2 (tree exp, rtx t
> if (optab_handler (builtin_optab, mode) == CODE_FOR_nothing)
> return NULL_RTX;
>
> - target = gen_reg_rtx (mode);
> + result = gen_reg_rtx (mode);
>
> if (! flag_errno_math || ! HONOR_NANS (mode))
> errno_set = false;
> @@ -2151,29 +2151,29 @@ expand_builtin_mathfn_2 (tree exp, rtx t
>
> start_sequence ();
>
> - /* Compute into TARGET.
> - Set TARGET to wherever the result comes back. */
> - target = expand_binop (mode, builtin_optab, op0, op1,
> - target, 0, OPTAB_DIRECT);
> + /* Compute into RESULT.
> + Set RESULT to wherever the result comes back. */
> + result = expand_binop (mode, builtin_optab, op0, op1,
> + result, 0, OPTAB_DIRECT);
>
> /* If we were unable to expand via the builtin, stop the sequence
> (without outputting the insns) and call to the library function
> with the stabilized argument list. */
> - if (target == 0)
> + if (result == 0)
> {
> end_sequence ();
> return expand_call (exp, target, target == const0_rtx);
> }
>
> if (errno_set)
> - expand_errno_check (exp, target);
> + expand_errno_check (exp, result);
>
> /* Output the entire sequence. */
> insns = get_insns ();
> end_sequence ();
> emit_insn (insns);
>
> - return target;
> + return result;
> }
>
> /* Expand a call to the builtin trinary math functions (fma).
> @@ -2187,7 +2187,7 @@ expand_builtin_mathfn_2 (tree exp, rtx t
> expand_builtin_mathfn_ternary (tree exp, rtx target, rtx subtarget)
> {
> optab builtin_optab;
> - rtx op0, op1, op2, insns;
> + rtx op0, op1, op2, insns, result;
> tree fndecl = get_callee_fndecl (exp);
> tree arg0, arg1, arg2;
> enum machine_mode mode;
> @@ -2214,7 +2214,7 @@ expand_builtin_mathfn_ternary (tree exp,
> if (optab_handler (builtin_optab, mode) == CODE_FOR_nothing)
> return NULL_RTX;
>
> - target = gen_reg_rtx (mode);
> + result = gen_reg_rtx (mode);
>
> /* Always stabilize the argument list. */
> CALL_EXPR_ARG (exp, 0) = arg0 = builtin_save_expr (arg0);
> @@ -2227,15 +2227,15 @@ expand_builtin_mathfn_ternary (tree exp,
>
> start_sequence ();
>
> - /* Compute into TARGET.
> - Set TARGET to wherever the result comes back. */
> - target = expand_ternary_op (mode, builtin_optab, op0, op1, op2,
> - target, 0);
> + /* Compute into RESULT.
> + Set RESULT to wherever the result comes back. */
> + result = expand_ternary_op (mode, builtin_optab, op0, op1, op2,
> + result, 0);
>
> /* If we were unable to expand via the builtin, stop the sequence
> (without outputting the insns) and call to the library function
> with the stabilized argument list. */
> - if (target == 0)
> + if (result == 0)
> {
> end_sequence ();
> return expand_call (exp, target, target == const0_rtx);
> @@ -2246,7 +2246,7 @@ expand_builtin_mathfn_ternary (tree exp,
> end_sequence ();
> emit_insn (insns);
>
> - return target;
> + return result;
> }
>
> /* Expand a call to the builtin sin and cos math functions.
> @@ -2298,7 +2298,7 @@ expand_builtin_mathfn_3 (tree exp, rtx t
> /* Before working hard, check whether the instruction is available. */
> if (optab_handler (builtin_optab, mode) != CODE_FOR_nothing)
> {
> - target = gen_reg_rtx (mode);
> + rtx result = gen_reg_rtx (mode);
>
> /* Wrap the computation of the argument in a SAVE_EXPR, as we may
> need to expand the argument again. This way, we will not perform
> @@ -2309,37 +2309,35 @@ expand_builtin_mathfn_3 (tree exp, rtx t
>
> start_sequence ();
>
> - /* Compute into TARGET.
> - Set TARGET to wherever the result comes back. */
> + /* Compute into RESULT.
> + Set RESULT to wherever the result comes back. */
> if (builtin_optab == sincos_optab)
> {
> - int result;
> + int ok;
>
> switch (DECL_FUNCTION_CODE (fndecl))
> {
> CASE_FLT_FN (BUILT_IN_SIN):
> - result = expand_twoval_unop (builtin_optab, op0, 0, target, 0);
> + ok = expand_twoval_unop (builtin_optab, op0, 0, result, 0);
> break;
> CASE_FLT_FN (BUILT_IN_COS):
> - result = expand_twoval_unop (builtin_optab, op0, target, 0, 0);
> + ok = expand_twoval_unop (builtin_optab, op0, result, 0, 0);
> break;
> default:
> gcc_unreachable ();
> }
> - gcc_assert (result);
> + gcc_assert (ok);
> }
> else
> - {
> - target = expand_unop (mode, builtin_optab, op0, target, 0);
> - }
> + result = expand_unop (mode, builtin_optab, op0, result, 0);
>
> - if (target != 0)
> + if (result != 0)
> {
> /* Output the entire sequence. */
> insns = get_insns ();
> end_sequence ();
> emit_insn (insns);
> - return target;
> + return result;
> }
>
> /* If we were unable to expand via the builtin, stop the sequence
> @@ -2348,9 +2346,7 @@ expand_builtin_mathfn_3 (tree exp, rtx t
> end_sequence ();
> }
>
> - target = expand_call (exp, target, target == const0_rtx);
> -
> - return target;
> + return expand_call (exp, target, target == const0_rtx);
> }
>
> /* Given an interclass math builtin decl FNDECL and it's argument ARG
> @@ -2820,7 +2816,7 @@ expand_builtin_int_roundingfn_2 (tree ex
> /* There's no easy way to detect the case we need to set EDOM. */
> if (!flag_errno_math)
> {
> - target = gen_reg_rtx (mode);
> + rtx result = gen_reg_rtx (mode);
>
> /* Wrap the computation of the argument in a SAVE_EXPR, as we may
> need to expand the argument again. This way, we will not perform
> @@ -2831,13 +2827,13 @@ expand_builtin_int_roundingfn_2 (tree ex
>
> start_sequence ();
>
> - if (expand_sfix_optab (target, op0, builtin_optab))
> + if (expand_sfix_optab (result, op0, builtin_optab))
> {
> /* Output the entire sequence. */
> insns = get_insns ();
> end_sequence ();
> emit_insn (insns);
> - return target;
> + return result;
> }
>
> /* If we were unable to expand via the builtin, stop the sequence
> @@ -2865,9 +2861,7 @@ expand_builtin_int_roundingfn_2 (tree ex
> return convert_to_mode (mode, target, 0);
> }
>
> - target = expand_call (exp, target, target == const0_rtx);
> -
> - return target;
> + return expand_call (exp, target, target == const0_rtx);
> }
>
> /* Expand a call to the powi built-in mathematical function. Return NULL_RTX if
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-12-30 19:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-23 17:02 RFA: Fix reuse of "target" variable in builtins.c Richard Sandiford
2012-12-30 19:24 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).