[PATCH] fix PR translation/82185

[PATCH] fix PR translation/82185

[Max Filippov]

2017-09-11  Max Filippov  <jcmvbkbc@gmail.com>
gcc/
	* expmed.c (emit_store_flag_int): Initialize rtx tem.
---
 gcc/expmed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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
-- 
2.1.4

Re: [PATCH] fix PR translation/82185

[Richard Sandiford]

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.  */

Re: [PATCH] fix PR translation/82185

[Max Filippov]

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.

-- 
Thanks.
-- Max

Re: [PATCH] fix PR translation/82185

[Jeff Law]

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