On Wed, 16 Nov 2016, Richard Biener wrote:

On Wed, 16 Nov 2016, Marc Glisse wrote:

On Wed, 16 Nov 2016, Richard Biener wrote:

On Wed, 16 Nov 2016, Marc Glisse wrote:

On Wed, 16 Nov 2016, Richard Biener wrote:

I am testing the following to avoid undefined behavior when negating
a multiplication (basically extending a previous fix to properly handle
negative power of two).

Bootstrap / regtest running on x86_64-unknown-linux-gnu.

Richard.

2016-11-16  Richard Biener  <rguent...@suse.de>

        PR middle-end/78305
        * fold-const.c (negate_expr_p): Fix multiplication case.

        * gcc.dg/torture/pr78305.c: New testcase.

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c    (revision 242471)
+++ gcc/fold-const.c    (working copy)
@@ -450,13 +450,15 @@ negate_expr_p (tree t)
      if (TYPE_UNSIGNED (type))
        break;
      /* INT_MIN/n * n doesn't overflow while negating one operand it
does
-         if n is a power of two.  */
+         if n is a power of (minus) two.  */

if n is (minus) a power of two.
if n is a divisor of INT_MIN.

n is a divisor of INT_MIN is correct.

      if (INTEGRAL_TYPE_P (TREE_TYPE (t))
          && ! TYPE_OVERFLOW_WRAPS (TREE_TYPE (t))
          && ! ((TREE_CODE (TREE_OPERAND (t, 0)) == INTEGER_CST
-                && ! integer_pow2p (TREE_OPERAND (t, 0)))
+                && (wi::popcount (TREE_OPERAND (t, 0))
+                    != 1 + wi::neg_p (TREE_OPERAND (t, 0), SIGNED)))

Is that supposed to test for (possibly negated) powers of 2? I don't see
it.
For -2, aka 0b11...110, popcount is 31 != 1 + 1.

It's supposed to test for a power of two with the sign-bit ORed in.
I believe those are which, when multiplied with some number can
yield INT_MIN.  That is, we look for a test that ensures that there
exists no number when multiplied with X yields INT_MIN.

The first sentence about ORing the sign bit sounds strange (except for a
sign-magnitude representation). With 2's complement, INT_MIN is -2^31, the
divisors are the 2^k and -(2^k). -2 * 2^30 yields INT_MIN, but your test
misses -2 as a possible divisor. On the other hand, 0b100...001 (aka -INT_MAX)
is not a divisor of INT_MIN but your test says the reverse.

Yeah, but it handled the testcase ;)  So I guess the easiest would be
to check integer_pow2p (abs (TREE_OPERAND (t, 0)) then, thus
wi::popcount (wi::abs (TREE_OPERAND (t, 0))) == 1?

Looks good to me, thanks.

--
Marc Glisse

Reply via email to