[Bug middle-end/33779] New: [4.3 Regression] folds unsigned multiplication == 0 to true

[Bug middle-end/33779] New: [4.3 Regression] folds unsigned multiplication == 0 to true

[rguenth at gcc dot gnu dot org]

int foo(int i)
{
  if (((unsigned)(i + 1)) * 4 == 0)
    return 1;
  return 0;
}

extern void abort(void);
int main()
{
  if (foo(0x3fffffff) == 0)
    abort ();
  return 0;
}


This goes wrong in extract_muldiv and is exposed by folding X * C CMP 0 to
X CMP 0 for undefined overflow.


-- 
           Summary: [4.3 Regression] folds unsigned multiplication == 0 to
                    true
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: rguenth at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[rguenth at gcc dot gnu dot org]

------- Comment #1 from rguenth at gcc dot gnu dot org  2007-10-15 13:24 -------
Another one:

int foo(int i)
{
  return ((int)((unsigned)(i + 1) * 4)) / 4;
}

extern void abort(void);
int main()
{
  if (foo(0x3fffffff) == 0)
    abort ();
  return 0;
}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2007-10-15 13:31 -------
whoops, make the testcase in comment #1

int foo(int i)
{
  return ((int)((unsigned)(i + 1) * 4)) / 4;
}

extern void abort(void);
int main()
{
  if (foo(0x3fffffff) != 0)
    abort ();
  return 0;
}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2007-10-15 13:32 -------
Only the first testcase is a regression (AFAIK), the second one also fails with
2.95.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.3.0
      Known to work|                            |4.2.2


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[janis at gcc dot gnu dot org]

------- Comment #4 from janis at gcc dot gnu dot org  2007-10-19 16:31 -------
A regression hunt for the first testcase on powerpc-linux identified the
following patch:

    http://gcc.gnu.org/viewcvs?view=rev&rev=120649

    r120649 | ian | 2007-01-10 21:07:38 +0000 (Wed, 10 Jan 2007)


-- 

janis at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ian at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org
           Severity|normal                      |critical
   Target Milestone|---                         |4.3.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[tromey at gcc dot gnu dot org]

------- Comment #5 from tromey at gcc dot gnu dot org  2007-10-27 16:42 -------
I looked at this a little.

This test is a little bit funny because the '+' has undefined
overflow but the '*' does not.  Move the cast to make the '+'
have defined overflow, and it works.

What happens is that we call fold_sign_changed_comparison
on the '==' expression (in the very first test case).
This decides that the expression can be simplified by
removing the casts to unsigned.  Some earlier call has already
hoisted the cast to unsigned to apply to the '*' tree as a
whole, so stripping this means we are optimizing a signed
multiply.

Offhand I would say that it is not valid to convert an unsigned
operation to a signed operation, because that changes overflow
from defined to undefined.  Going the other direction should
generally be ok -- though, I suppose, sometimes miss an optimization.

Removing the special case for EQ_EXPR and NE_EXPR from
fold_sign_changed_comparison should fix this bug, but I don't know
at what cost.  Or, perhaps this test could be changed to look at
whether arg0_inner is now a signed binary expression.  Some advice
from a fold expert would be appreciated.


-- 

tromey at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-10-27 16:42:51
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2007-10-27 17:29 -------
"This goes wrong in extract_muldiv..."

sorry for not pasting my complete analysis (actually the testcase is carefuly
crafted from looking at this code ;)).  The recursion through conversions

    case CONVERT_EXPR:  case NON_LVALUE_EXPR:  case NOP_EXPR:
      /* If op0 is an expression ...  */
      if ((COMPARISON_CLASS_P (op0)
           || UNARY_CLASS_P (op0)
           || BINARY_CLASS_P (op0)
           || VL_EXP_CLASS_P (op0)
           || EXPRESSION_CLASS_P (op0))
          /* ... and is unsigned, and its type is smaller than ctype,
             then we cannot pass through as widening.  */
          && ((TYPE_UNSIGNED (TREE_TYPE (op0))
               && ! (TREE_CODE (TREE_TYPE (op0)) == INTEGER_TYPE
                     && TYPE_IS_SIZETYPE (TREE_TYPE (op0)))
               && (GET_MODE_SIZE (TYPE_MODE (ctype))
                   > GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0)))))
              /* ... or this is a truncation (t is narrower than op0),
                 then we cannot pass through this narrowing.  */
              || (GET_MODE_SIZE (TYPE_MODE (type))
                  < GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0))))
              /* ... or signedness changes for division or modulus,
                 then we cannot pass through this conversion.  */
              || (code != MULT_EXPR
                  && (TYPE_UNSIGNED (ctype)
                      != TYPE_UNSIGNED (TREE_TYPE (op0))))))
        break;

      /* Pass the constant down and see if we can make a simplification.  If
         we can, replace this expression with the inner simplification for
         possible later conversion to our or some other type.  */
      if ((t2 = fold_convert (TREE_TYPE (op0), c)) != 0
          && TREE_CODE (t2) == INTEGER_CST
          && !TREE_OVERFLOW (t2)
          && (0 != (t1 = extract_muldiv (op0, t2, code,
                                         code == MULT_EXPR
                                         ? ctype : NULL_TREE,
                                         strict_overflow_p))))
        return t1;
      break;

is wrong as it might convert the transformation from one in unsigned type
to one in signed type.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[rguenth at gcc dot gnu dot org]

------- Comment #7 from rguenth at gcc dot gnu dot org  2007-10-27 17:30 -------
I'll take this.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rguenth at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2007-10-27 16:42:51         |2007-10-27 17:30:27
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[rguenth at gcc dot gnu dot org]

------- Comment #8 from rguenth at gcc dot gnu dot org  2007-10-31 12:33 -------
Subject: Bug 33779

Author: rguenth
Date: Wed Oct 31 12:33:05 2007
New Revision: 129796

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=129796
Log:
2007-10-31  Richard Guenther  <rguenther@suse.de>

        PR middle-end/33779
        * fold-const.c (extract_muldiv_1): Make sure to not introduce
        new undefined integer overflow.
        (fold_binary): Avoid useless conversion.

        * gcc.c-torture/execute/pr33779-1.c: New testcase.
        * gcc.c-torture/execute/pr33779-2.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr33779-1.c
    trunk/gcc/testsuite/gcc.c-torture/execute/pr33779-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779

[Bug middle-end/33779] [4.3 Regression] folds unsigned multiplication == 0 to true

[rguenth at gcc dot gnu dot org]

------- Comment #9 from rguenth at gcc dot gnu dot org  2007-10-31 12:34 -------
Fixed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33779