PR 61136: wide-int fallout in int_const_binop_1

The wide-int version of int_const_binop_1 has:

static tree
int_const_binop_1 (enum tree_code code, const_tree arg1, const_tree parg2,
		   int overflowable)
{
  [...]
  tree type = TREE_TYPE (arg1);
  [...]
  wide_int arg2 = wide_int::from (parg2, TYPE_PRECISION (type),
				  TYPE_SIGN (TREE_TYPE (parg2)));

The idea that the result should be the same type as arg1 predates
wide-int but the conversion of arg2 to that type is new.

The new conversion can't be right because it loses overflow information.
The question then is whether this function should be prepared to handle
mismatched arguments (as the pre-wide-int version effectively did, since
it did everything in double_int precision) or whether the operation
should be strictly typed.  If the former then the function should be
doing the calculation in widest_int, which would make it a much more
direct analogue of the original code.  If the latter then we should be
able to operate directly on arg2.

I assume it really is supposed to be strictly typed though.  It seems
strange to use the type of the first operand as the type of the result
otherwise.  E.g. in one of the cases I've found where it isn't strictly
typed we create:

 <mult_expr 0x7ffff1310758
    type <integer_type 0x7ffff11ec888 long unsigned int public unsigned DI
        size <integer_cst 0x7ffff11daf78 constant 64>
        unit size <integer_cst 0x7ffff11daf90 constant 8>
        align 64 symtab 0 alias set -1 canonical type 0x7ffff11ec888 precision 64 min <integer_cst 0x7ffff11f9258 0> max <integer_cst 0x7ffff11f0100 18446744073709551615>>

    arg 0 <integer_cst 0x7ffff11f9720 type <integer_type 0x7ffff11ec690 int> constant 4>
    arg 1 <sizeof_expr 0x7ffff130d280 type <integer_type 0x7ffff11ec888 long unsigned int>
        readonly
        arg 0 <integer_type 0x7ffff11ec690 int public type_6 SI
            size <integer_cst 0x7ffff11f91c8 constant 32>
            unit size <integer_cst 0x7ffff11f91e0 constant 4>
            align 32 symtab 0 alias set -1 canonical type 0x7ffff11ec690 precision 32 min <integer_cst 0x7ffff11f9180 -2147483648> max <integer_cst 0x7ffff11f9198 2147483647>
            pointer_to_this <pointer_type 0x7ffff1200738>>
        ../../../gcc/gcc/testsuite/g++.dg/gomp/declare-simd-1.C:16:62>
    ../../../gcc/gcc/testsuite/g++.dg/gomp/declare-simd-1.C:16:53>

and these arguments later find their way to int_const_binop_1.
The result and second argument are long (64 bits) but the first
argument is int (32 bits).  So a simple conversion to widest_int
would mean that the result would be 32 bits rather than 64 bits,
which doesn't look intentional.

I tried making int_const_binop_1 strictly typed and the only thing
that was needed to pass bootstrap was a fairly obvious-looking patch
to the Ada frontend.  But there's quite a bit of fallout in the testsuite
itself (including the above).  I'm still working my way through that,
so please shout if I'm going doing a rathole.

This causes a problem in the testcase because of this code in
multiple_of_p:

    case INTEGER_CST:
      if (TREE_CODE (bottom) != INTEGER_CST
	  || integer_zerop (bottom)
	  || (TYPE_UNSIGNED (type)
	      && (tree_int_cst_sgn (top) < 0
		  || tree_int_cst_sgn (bottom) < 0)))
	return 0;
      return integer_zerop (int_const_binop (TRUNC_MOD_EXPR,
					     top, bottom));

Here TOP and BOTTOM aren't necessarily the same type.  In the testcase
for PR61136 BOTTOM is wider than TOP and when converted (truncated)
to TOP's type becomes zero.  This then falls foul of the null escape in:

    case TRUNC_MOD_EXPR:
      if (arg2 == 0)
	return NULL_TREE;
      res = wi::mod_trunc (arg1, arg2, sign, &overflow);
      break;

So we end up calling integer_zerop on a null tree.

We know at the point of the int_const_binop call that we're dealing
with two INTEGER_CSTs, so it seems silly to create a tree for the result
only to test whether it's zero.  This patch therefore uses wi:: directly.
I think this is worth doing independently of the eventual patches to fix
the type incompatibilities elsewhere.

The test is "wi::smod_trunc (...) == 0" but it seemed more mnemonic to
have a version of wi::multiple_of_p that doesn't compute the quotient.

Tested on x86_64-linux-gnu.  OK to install?

Thanks,
Richard

gcc/
	PR tree-optimization/61136
	* wide-int.h (multiple_of_p): Define a version that doesn't return
	the quotient.
	* fold-const.c (extract_muldiv_1): Use wi::multiple_of_p instead of an
	integer_zerop/const_binop pair.
	(multiple_of_p): Likewise, converting both operands to widest_int
	precision.

gcc/testsuite/
	* gcc.dg/torture/pr61136.c: New test.

PR 61136: wide-int fallout in int_const_binop_1

Commit Message

Comments

Patch