Fix combine's simplify_comparison (PR rtl-optimization/51023)

Hi!

This patch essentially reverts part of Bernd's 2011-07-06 changes,
which was IMHO wrong.  As const_op here is a constant in wider mode than
MODE (which is the inner mode of the SIGN_EXTEND), the old code
(and what this patch is restoring) didn't check just that the sign bit
is clear, but also that all bits above the sign bit of MODE are clear too.
I've used GET_MODE_PRECISION instead of GET_MODE_BITSIZE (as changed
in many other places around that date) and HWI_COMPUTABLE_MODE_P macro.

Additionally, if GET_MODE_CLASS (mode) == MODE_INT, we know
mode != VOIDmode, there is no point testing it (but the compiler will
likely not know it, as GET_MODE_CLASS is reading from an array).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

BTW, the
@@ -6546,10 +6551,8 @@ simplify_set (rtx x)
       enum machine_mode inner_mode = GET_MODE (inner);
 
       /* Here we make sure that we don't have a sign bit on.  */
-      if (GET_MODE_BITSIZE (inner_mode) <= HOST_BITS_PER_WIDE_INT
-         && (nonzero_bits (inner, inner_mode)
-             < ((unsigned HOST_WIDE_INT) 1
-                << (GET_MODE_BITSIZE (GET_MODE (src)) - 1))))
+      if (val_signbit_known_clear_p (GET_MODE (src),
+                                    nonzero_bits (inner, inner_mode)))
        {
          SUBST (SET_SRC (x), inner);
          src = SET_SRC (x);
hunk from the same patch might have the same problem (can't nonzero_bits
be wider than the mode of src there?), but I don't have a testcase for that.
Can you please double check that?

2011-11-09  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/51023
	* combine.c (simplify_comparison) <case SIGN_EXTEND>: Don't use
	val_signbit_known_clear_p for signed comparison narrowing
	optimization.  Don't check for non-VOIDmode, use
	HWI_COMPUTABLE_MODE_P macro.
	<case ZERO_EXTEND>: Don't check for non-VOIDmode.

	* gcc.c-torture/execute/pr51023.c: New test.


	Jakub

On 11/09/11 16:32, Jakub Jelinek wrote:
> --- gcc/combine.c.jj	2011-11-08 23:35:12.000000000 +0100
> +++ gcc/combine.c	2011-11-09 10:06:27.207333364 +0100
> @@ -11397,9 +11397,12 @@ simplify_comparison (enum rtx_code code,
>  	     later on, and then we wouldn't know whether to sign- or
>  	     zero-extend.  */
>  	  mode = GET_MODE (XEXP (op0, 0));
> -	  if (mode != VOIDmode && GET_MODE_CLASS (mode) == MODE_INT
> +	  if (GET_MODE_CLASS (mode) == MODE_INT
>  	      && ! unsigned_comparison_p
> -	      && val_signbit_known_clear_p (mode, const_op)
> +	      && HWI_COMPUTABLE_MODE_P (mode)
> +	      && ((unsigned HOST_WIDE_INT) const_op
> +		  < (((unsigned HOST_WIDE_INT) 1
> +		      << (GET_MODE_PRECISION (mode) - 1))))
>  	      && have_insn_for (COMPARE, mode))
>  	    {
>  	      op0 = XEXP (op0, 0);

I guess this is OK, although it would still be nicer to use something
with a descriptive name rather than bit-fiddling, to avoid this kind of
confusion. Maybe check whether trunc_int_for_mode doesn't change the
constant? Or we could make val_signbit_known_clear_p do more extensive
checking.


Bernd