This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix divmod expansion (PR middle-end/78416, take 2)
On Mon, Nov 21, 2016 at 8:09 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On Fri, Nov 18, 2016 at 11:10:58PM +0100, Richard Biener wrote:
>> I wonder if transforming the const-int to wide int makes this all easier to read?
>
> Here is updated patch that does that.
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Richard.
> 2016-11-21 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/78416
> * expmed.c (expand_divmod): Use wide_int for computation of
> op1_is_pow2. Don't set it if op1 is 0. Formatting fixes.
> Use size <= HOST_BITS_PER_WIDE_INT instead of
> HOST_BITS_PER_WIDE_INT >= size.
>
> * gcc.dg/torture/pr78416.c: New test.
>
> --- gcc/expmed.c.jj 2016-11-19 18:02:45.431380371 +0100
> +++ gcc/expmed.c 2016-11-21 16:13:32.980271174 +0100
> @@ -3994,11 +3994,10 @@ expand_divmod (int rem_flag, enum tree_c
> op1_is_constant = CONST_INT_P (op1);
> if (op1_is_constant)
> {
> - unsigned HOST_WIDE_INT ext_op1 = UINTVAL (op1);
> - if (unsignedp)
> - ext_op1 &= GET_MODE_MASK (mode);
> - op1_is_pow2 = ((EXACT_POWER_OF_2_OR_ZERO_P (ext_op1)
> - || (! unsignedp && EXACT_POWER_OF_2_OR_ZERO_P (-ext_op1))));
> + wide_int ext_op1 = rtx_mode_t (op1, mode);
> + op1_is_pow2 = (wi::popcount (ext_op1) == 1
> + || (! unsignedp
> + && wi::popcount (wi::neg (ext_op1)) == 1));
> }
>
> /*
> @@ -4079,11 +4078,10 @@ expand_divmod (int rem_flag, enum tree_c
> not straightforward to generalize this. Maybe we should make an array
> of possible modes in init_expmed? Save this for GCC 2.7. */
>
> - optab1 = ((op1_is_pow2 && op1 != const0_rtx)
> + optab1 = (op1_is_pow2
> ? (unsignedp ? lshr_optab : ashr_optab)
> : (unsignedp ? udiv_optab : sdiv_optab));
> - optab2 = ((op1_is_pow2 && op1 != const0_rtx)
> - ? optab1
> + optab2 = (op1_is_pow2 ? optab1
> : (unsignedp ? udivmod_optab : sdivmod_optab));
>
> for (compute_mode = mode; compute_mode != VOIDmode;
> @@ -4139,10 +4137,15 @@ expand_divmod (int rem_flag, enum tree_c
> /* convert_modes may have placed op1 into a register, so we
> must recompute the following. */
> op1_is_constant = CONST_INT_P (op1);
> - op1_is_pow2 = (op1_is_constant
> - && ((EXACT_POWER_OF_2_OR_ZERO_P (INTVAL (op1))
> - || (! unsignedp
> - && EXACT_POWER_OF_2_OR_ZERO_P (-UINTVAL (op1))))));
> + if (op1_is_constant)
> + {
> + wide_int ext_op1 = rtx_mode_t (op1, compute_mode);
> + op1_is_pow2 = (wi::popcount (ext_op1) == 1
> + || (! unsignedp
> + && wi::popcount (wi::neg (ext_op1)) == 1));
> + }
> + else
> + op1_is_pow2 = 0;
> }
>
> /* If one of the operands is a volatile MEM, copy it into a register. */
> @@ -4182,10 +4185,10 @@ expand_divmod (int rem_flag, enum tree_c
> unsigned HOST_WIDE_INT mh, ml;
> int pre_shift, post_shift;
> int dummy;
> - unsigned HOST_WIDE_INT d = (INTVAL (op1)
> - & GET_MODE_MASK (compute_mode));
> + wide_int wd = rtx_mode_t (op1, compute_mode);
> + unsigned HOST_WIDE_INT d = wd.to_uhwi ();
>
> - if (EXACT_POWER_OF_2_OR_ZERO_P (d))
> + if (wi::popcount (wd) == 1)
> {
> pre_shift = floor_log2 (d);
> if (rem_flag)
> @@ -4325,7 +4328,7 @@ expand_divmod (int rem_flag, enum tree_c
> else if (d == -1)
> quotient = expand_unop (compute_mode, neg_optab, op0,
> tquotient, 0);
> - else if (HOST_BITS_PER_WIDE_INT >= size
> + else if (size <= HOST_BITS_PER_WIDE_INT
> && abs_d == HOST_WIDE_INT_1U << (size - 1))
> {
> /* This case is not handled correctly below. */
> @@ -4335,6 +4338,7 @@ expand_divmod (int rem_flag, enum tree_c
> goto fail1;
> }
> else if (EXACT_POWER_OF_2_OR_ZERO_P (d)
> + && (size <= HOST_BITS_PER_WIDE_INT || d >= 0)
> && (rem_flag
> ? smod_pow2_cheap (speed, compute_mode)
> : sdiv_pow2_cheap (speed, compute_mode))
> @@ -4348,7 +4352,9 @@ expand_divmod (int rem_flag, enum tree_c
> compute_mode)
> != CODE_FOR_nothing)))
> ;
> - else if (EXACT_POWER_OF_2_OR_ZERO_P (abs_d))
> + else if (EXACT_POWER_OF_2_OR_ZERO_P (abs_d)
> + && (size <= HOST_BITS_PER_WIDE_INT
> + || abs_d != (unsigned HOST_WIDE_INT) d))
> {
> if (rem_flag)
> {
> @@ -4483,7 +4489,7 @@ expand_divmod (int rem_flag, enum tree_c
> case FLOOR_DIV_EXPR:
> case FLOOR_MOD_EXPR:
> /* We will come here only for signed operations. */
> - if (op1_is_constant && HOST_BITS_PER_WIDE_INT >= size)
> + if (op1_is_constant && size <= HOST_BITS_PER_WIDE_INT)
> {
> unsigned HOST_WIDE_INT mh, ml;
> int pre_shift, lgup, post_shift;
> @@ -4552,9 +4558,8 @@ expand_divmod (int rem_flag, enum tree_c
> op0, constm1_rtx), NULL_RTX);
> t2 = expand_binop (compute_mode, ior_optab, op0, t1, NULL_RTX,
> 0, OPTAB_WIDEN);
> - nsign = expand_shift
> - (RSHIFT_EXPR, compute_mode, t2,
> - size - 1, NULL_RTX, 0);
> + nsign = expand_shift (RSHIFT_EXPR, compute_mode, t2,
> + size - 1, NULL_RTX, 0);
> t3 = force_operand (gen_rtx_MINUS (compute_mode, t1, nsign),
> NULL_RTX);
> t4 = expand_divmod (0, TRUNC_DIV_EXPR, compute_mode, t3, op1,
> @@ -4663,7 +4668,10 @@ expand_divmod (int rem_flag, enum tree_c
> case CEIL_MOD_EXPR:
> if (unsignedp)
> {
> - if (op1_is_constant && EXACT_POWER_OF_2_OR_ZERO_P (INTVAL (op1)))
> + if (op1_is_constant
> + && EXACT_POWER_OF_2_OR_ZERO_P (INTVAL (op1))
> + && (size <= HOST_BITS_PER_WIDE_INT
> + || INTVAL (op1) >= 0))
> {
> rtx t1, t2, t3;
> unsigned HOST_WIDE_INT d = INTVAL (op1);
> @@ -4876,7 +4884,7 @@ expand_divmod (int rem_flag, enum tree_c
> break;
>
> case EXACT_DIV_EXPR:
> - if (op1_is_constant && HOST_BITS_PER_WIDE_INT >= size)
> + if (op1_is_constant && size <= HOST_BITS_PER_WIDE_INT)
> {
> HOST_WIDE_INT d = INTVAL (op1);
> unsigned HOST_WIDE_INT ml;
> --- gcc/testsuite/gcc.dg/torture/pr78416.c.jj 2016-11-21 14:34:50.830177377 +0100
> +++ gcc/testsuite/gcc.dg/torture/pr78416.c 2016-11-21 14:34:50.830177377 +0100
> @@ -0,0 +1,17 @@
> +/* PR middle-end/78416 */
> +/* { dg-do run { target int128 } } */
> +
> +int
> +main ()
> +{
> + unsigned __int128 x;
> + x = 0xFFFFFFFFFFFFFFFFULL;
> + x /= ~0x7FFFFFFFFFFFFFFFLL;
> + if (x != 0)
> + __builtin_abort ();
> + x = ~0x7FFFFFFFFFFFFFFELL;
> + x /= ~0x7FFFFFFFFFFFFFFFLL;
> + if (x != 1)
> + __builtin_abort ();
> + return 0;
> +}
>
>
> Jakub