diff mbox

[3/3,RTL,ifcvt] PR middle-end/37780: Conditional expression with __builtin_clz() should be optimized out

Message ID 5756FAA0.7080905@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov June 7, 2016, 4:47 p.m. UTC 
On 07/06/16 08:37, Andreas Schwab wrote:
> Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> writes:
>
>> +static rtx
>> +simplify_cond_clz_ctz (rtx x, rtx_code cmp_code, rtx true_val, rtx false_val)
>> +{
>> +  if (cmp_code != EQ && cmp_code != NE)
>> +    return NULL_RTX;
>> +
>> +  /* Result on X == 0 and X !=0 respectively.  */
>> +  rtx on_zero, on_nonzero;
>> +  if (cmp_code == EQ)
>> +    {
>> +      on_zero = true_val;
>> +      on_nonzero = false_val;
>> +    }
>> +  else
>> +    {
>> +      on_zero = false_val;
>> +      on_nonzero = true_val;
>> +    }
>> +
>> +  rtx_code op_code = GET_CODE (on_nonzero);
>> +  if ((op_code != CLZ && op_code != CTZ)
>> +      || !rtx_equal_p (XEXP (on_nonzero, 0), x)
>> +      || !CONST_INT_P (on_zero))
>> +    return NULL_RTX;
>> +
>> +  HOST_WIDE_INT op_val;
>> +  machine_mode mode = GET_MODE (on_nonzero);
>> +  if (((op_code == CLZ && CLZ_DEFINED_VALUE_AT_ZERO (mode, op_val))
>> +	|| (op_code == CTZ && CTZ_DEFINED_VALUE_AT_ZERO (mode, op_val)))
>> +      && op_val == INTVAL (on_zero))
>> +    return on_nonzero;
>> +
>> +  return NULL_RTX;
>> +}
> ../../gcc/simplify-rtx.c: In function 'rtx_def* simplify_cond_clz_ctz(rtx, rtx_code, rtx, rtx)':
> ../../gcc/simplify-rtx.c:5304:16: error: unused variable 'mode' [-Werror=unused-variable]
>     machine_mode mode = GET_MODE (on_nonzero);
>                  ^~~~
>
> Andreas.

That must be on targets that don't define CLZ_DEFINED_VALUE_AT_ZERO or CTZ_DEFINED_VALUE_AT_ZERO.
defaults.h then defines them to just "0".
I think GCC shouldn't warn in these cases as 'mode' is used in the function
it's defined, it's just the macro where it's used as an argument discards it.
Anyway, this patch removes that variable and fixes the warning.

Tested on arm-none-eabi after manually undefining CLZ_DEFINED_VALUE_AT_ZERO and CTZ_DEFINED_VALUE_AT_ZERO to reproduce
the warning and checking that it's fixed.
Committing as obvious in the interest of fixing bootstrap on the relevant targets.

Thanks,
Kyrill

Comments

Kyrill Tkachov June 7, 2016, 4:51 p.m. UTC | #1 
On 07/06/16 17:47, Kyrill Tkachov wrote:
>
> On 07/06/16 08:37, Andreas Schwab wrote:
>> Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> writes:
>>
>>> +static rtx
>>> +simplify_cond_clz_ctz (rtx x, rtx_code cmp_code, rtx true_val, rtx false_val)
>>> +{
>>> +  if (cmp_code != EQ && cmp_code != NE)
>>> +    return NULL_RTX;
>>> +
>>> +  /* Result on X == 0 and X !=0 respectively.  */
>>> +  rtx on_zero, on_nonzero;
>>> +  if (cmp_code == EQ)
>>> +    {
>>> +      on_zero = true_val;
>>> +      on_nonzero = false_val;
>>> +    }
>>> +  else
>>> +    {
>>> +      on_zero = false_val;
>>> +      on_nonzero = true_val;
>>> +    }
>>> +
>>> +  rtx_code op_code = GET_CODE (on_nonzero);
>>> +  if ((op_code != CLZ && op_code != CTZ)
>>> +      || !rtx_equal_p (XEXP (on_nonzero, 0), x)
>>> +      || !CONST_INT_P (on_zero))
>>> +    return NULL_RTX;
>>> +
>>> +  HOST_WIDE_INT op_val;
>>> +  machine_mode mode = GET_MODE (on_nonzero);
>>> +  if (((op_code == CLZ && CLZ_DEFINED_VALUE_AT_ZERO (mode, op_val))
>>> +    || (op_code == CTZ && CTZ_DEFINED_VALUE_AT_ZERO (mode, op_val)))
>>> +      && op_val == INTVAL (on_zero))
>>> +    return on_nonzero;
>>> +
>>> +  return NULL_RTX;
>>> +}
>> ../../gcc/simplify-rtx.c: In function 'rtx_def* simplify_cond_clz_ctz(rtx, rtx_code, rtx, rtx)':
>> ../../gcc/simplify-rtx.c:5304:16: error: unused variable 'mode' [-Werror=unused-variable]
>>     machine_mode mode = GET_MODE (on_nonzero);
>>                  ^~~~
>>
>> Andreas.
>
> That must be on targets that don't define CLZ_DEFINED_VALUE_AT_ZERO or CTZ_DEFINED_VALUE_AT_ZERO.
> defaults.h then defines them to just "0".
> I think GCC shouldn't warn in these cases as 'mode' is used in the function
> it's defined, it's just the macro where it's used as an argument discards it.
> Anyway, this patch removes that variable and fixes the warning.
>
> Tested on arm-none-eabi after manually undefining CLZ_DEFINED_VALUE_AT_ZERO and CTZ_DEFINED_VALUE_AT_ZERO to reproduce
> the warning and checking that it's fixed.
> Committing as obvious in the interest of fixing bootstrap on the relevant targets.
>

With the ChangeLog entry:

2016-06-07  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * simplify-rtx.c (simplify_cond_clz_ctz): Delete 'mode' local
     variable.

> Thanks,
> Kyrill
>
diff mbox

Patch

diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 0f750bb08e817ba630d92566c6a991f81e095304..21adfcb531f3f3379653c67145ab0023c978fd82 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -5314,9 +5314,10 @@  simplify_cond_clz_ctz (rtx x, rtx_code cmp_code, rtx true_val, rtx false_val)
     return NULL_RTX;
 
   HOST_WIDE_INT op_val;
-  machine_mode mode = GET_MODE (on_nonzero);
-  if (((op_code == CLZ && CLZ_DEFINED_VALUE_AT_ZERO (mode, op_val))
-	|| (op_code == CTZ && CTZ_DEFINED_VALUE_AT_ZERO (mode, op_val)))
+  if (((op_code == CLZ
+	&& CLZ_DEFINED_VALUE_AT_ZERO (GET_MODE (on_nonzero), op_val))
+      || (op_code == CTZ
+	  && CTZ_DEFINED_VALUE_AT_ZERO (GET_MODE (on_nonzero), op_val)))
       && op_val == INTVAL (on_zero))
     return on_nonzero;