Message ID | 1505164538-25675-1-git-send-email-jcmvbkbc@gmail.com |
---|---|
State | New |
Headers | show |
Series | fix PR translation/82185 | expand |
Max Filippov <jcmvbkbc@gmail.com> writes: > 2017-09-11 Max Filippov <jcmvbkbc@gmail.com> > gcc/ > * expmed.c (emit_store_flag_int): Initialize rtx tem. LGTM, thanks, but I can't approve it. This makes the later "tem = 0;" redundant, so perhaps it would make sense to delete that too? There again, it was redundant before the split as well. An alternative would be to only test tem when we've done something with it, as below, but I don't know if that's better or a step backwards. Thanks, Richard gcc/ * expmed.c (emit_store_flag_int): Only test tem if it has been initialized. Index: gcc/expmed.c =================================================================== --- gcc/expmed.c (revision 251980) +++ gcc/expmed.c (working copy) @@ -5601,7 +5601,6 @@ emit_store_flag_int (rtx target, rtx sub { machine_mode target_mode = target ? GET_MODE (target) : VOIDmode; rtx_insn *last = get_last_insn (); - rtx tem; /* If this is an equality comparison of integers, we can try to exclusive-or (or subtract) the two operands and use a recursive call to try the @@ -5610,8 +5609,8 @@ emit_store_flag_int (rtx target, rtx sub if ((code == EQ || code == NE) && op1 != const0_rtx) { - tem = expand_binop (mode, xor_optab, op0, op1, subtarget, 1, - OPTAB_WIDEN); + rtx tem = expand_binop (mode, xor_optab, op0, op1, subtarget, 1, + OPTAB_WIDEN); if (tem == 0) tem = expand_binop (mode, sub_optab, op0, op1, subtarget, 1, @@ -5643,26 +5642,28 @@ emit_store_flag_int (rtx target, rtx sub && rtx_cost (GEN_INT (normalizep), mode, PLUS, 1, optimize_insn_for_speed_p ()) == 0) { - tem = emit_store_flag_1 (subtarget, rcode, op0, op1, mode, 0, - STORE_FLAG_VALUE, target_mode); + rtx tem = emit_store_flag_1 (subtarget, rcode, op0, op1, mode, 0, + STORE_FLAG_VALUE, target_mode); if (tem != 0) tem = expand_binop (target_mode, add_optab, tem, gen_int_mode (normalizep, target_mode), target, 0, OPTAB_WIDEN); + if (tem != 0) + return tem; } else if (!want_add && rtx_cost (trueval, mode, XOR, 1, optimize_insn_for_speed_p ()) == 0) { - tem = emit_store_flag_1 (subtarget, rcode, op0, op1, mode, 0, - normalizep, target_mode); + rtx tem = emit_store_flag_1 (subtarget, rcode, op0, op1, mode, 0, + normalizep, target_mode); if (tem != 0) tem = expand_binop (target_mode, xor_optab, tem, trueval, target, INTVAL (trueval) >= 0, OPTAB_WIDEN); + if (tem != 0) + return tem; } - if (tem != 0) - return tem; delete_insns_since (last); } @@ -5680,7 +5681,7 @@ emit_store_flag_int (rtx target, rtx sub /* Try to put the result of the comparison in the sign bit. Assume we can't do the necessary operation below. */ - tem = 0; + rtx tem = 0; /* To see if A <= 0, compute (A | (A - 1)). A <= 0 iff that result has the sign bit set. */
Hi Richard, On Mon, Sep 11, 2017 at 2:36 PM, Richard Sandiford <richard.sandiford@linaro.org> wrote: > Max Filippov <jcmvbkbc@gmail.com> writes: >> 2017-09-11 Max Filippov <jcmvbkbc@gmail.com> >> gcc/ >> * expmed.c (emit_store_flag_int): Initialize rtx tem. > > LGTM, thanks, but I can't approve it. > > This makes the later "tem = 0;" redundant, so perhaps it would make > sense to delete that too? There again, it was redundant before the > split as well. > > An alternative would be to only test tem when we've done something > with it, as below, but I don't know if that's better or a step backwards. this works for me too, so whichever fix you like better.
On 09/11/2017 03:59 PM, Max Filippov wrote: > Hi Richard, > > On Mon, Sep 11, 2017 at 2:36 PM, Richard Sandiford > <richard.sandiford@linaro.org> wrote: >> Max Filippov <jcmvbkbc@gmail.com> writes: >>> 2017-09-11 Max Filippov <jcmvbkbc@gmail.com> >>> gcc/ >>> * expmed.c (emit_store_flag_int): Initialize rtx tem. >> >> LGTM, thanks, but I can't approve it. >> >> This makes the later "tem = 0;" redundant, so perhaps it would make >> sense to delete that too? There again, it was redundant before the >> split as well. >> >> An alternative would be to only test tem when we've done something >> with it, as below, but I don't know if that's better or a step backwards. > > this works for me too, so whichever fix you like better. I like narrowing the scope better -- it's a lot easier to reason about the code when the def and uses are close and there's not a ton of control flow. Jeff
diff --git a/gcc/expmed.c b/gcc/expmed.c index 7f0cb0a0ec05..945ab3d656a2 100644 --- a/gcc/expmed.c +++ b/gcc/expmed.c @@ -5601,7 +5601,7 @@ emit_store_flag_int (rtx target, rtx subtarget, enum rtx_code code, rtx op0, { machine_mode target_mode = target ? GET_MODE (target) : VOIDmode; rtx_insn *last = get_last_insn (); - rtx tem; + rtx tem = NULL_RTX; /* If this is an equality comparison of integers, we can try to exclusive-or (or subtract) the two operands and use a recursive call to try the