On November 18, 2016 10:45:10 PM GMT+01:00, Jakub Jelinek <ja...@redhat.com> wrote: >Hi! > >As the testcase shows, expand_divmod doesn't handle properly >division/remainder in modes larger than HOST_BITS_PER_WIDE_INT like >TImode - if op1 is "negative" CONST_INT, it means it has 65 most >significant >bits set, but EXACT_POWER_OF_2_OR_ZERO_P will happily return true on >that >and optimize the division into a right shift. >Fixed by taking into account that EXACT_POWER_OF_2_OR_ZERO_P works only >if either the mode bitsize/precision are smaller or equal than HWI, or >if the value doesn't have MSB bit set. > >Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
I wonder if transforming the const-int to wide int makes this all easier to read? >2016-11-18 Jakub Jelinek <ja...@redhat.com> > > PR middle-end/78416 > * expmed.c (expand_divmod): For modes wider than HWI, take into > account implicit 1 bits above msb for EXACT_POWER_OF_2_OR_ZERO_P. > 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-10-31 13:28:12.000000000 +0100 >+++ gcc/expmed.c 2016-11-18 16:57:57.608703605 +0100 >@@ -3998,7 +3998,15 @@ expand_divmod (int rem_flag, enum tree_c > 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)))); >+ /* If mode is wider than HWI and op1 has msb set, >+ then it has there extra implicit 1 bits above it. */ >+ && (GET_MODE_PRECISION (mode) <= HOST_BITS_PER_WIDE_INT >+ || INTVAL (op1) >= 0)) >+ || (! unsignedp >+ && EXACT_POWER_OF_2_OR_ZERO_P (-ext_op1) >+ && (GET_MODE_PRECISION (mode) >+ <= HOST_BITS_PER_WIDE_INT >+ || INTVAL (op1) < 0))); > } > > /* >@@ -4141,8 +4149,17 @@ expand_divmod (int rem_flag, enum tree_c > 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 mode is wider than HWI and op1 has msb set, >+ then it has there extra implicit 1 bits above >+ it. */ >+ && (GET_MODE_PRECISION (compute_mode) >+ <= HOST_BITS_PER_WIDE_INT >+ || INTVAL (op1) >= 0)) >+ || (! unsignedp >+ && EXACT_POWER_OF_2_OR_ZERO_P (-UINTVAL (op1)) >+ && (GET_MODE_PRECISION (compute_mode) >+ <= HOST_BITS_PER_WIDE_INT >+ || INTVAL (op1) < 0)))); > } > >/* If one of the operands is a volatile MEM, copy it into a register. >*/ >@@ -4185,7 +4202,8 @@ expand_divmod (int rem_flag, enum tree_c > unsigned HOST_WIDE_INT d = (INTVAL (op1) > & GET_MODE_MASK (compute_mode)); > >- if (EXACT_POWER_OF_2_OR_ZERO_P (d)) >+ if (EXACT_POWER_OF_2_OR_ZERO_P (d) >+ && (INTVAL (op1) >= 0 || size <= HOST_BITS_PER_WIDE_INT)) > { > pre_shift = floor_log2 (d); > if (rem_flag) >@@ -4325,7 +4343,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 +4353,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 +4367,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 +4504,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 +4573,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 +4683,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 +4899,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-18 >16:43:35.010448325 +0100 >+++ gcc/testsuite/gcc.dg/torture/pr78416.c 2016-11-18 >16:44:40.388634204 +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