[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

--- Comment #11 from Richard Biener  ---
Author: rguenth
Date: Tue Apr  4 09:06:04 2017
New Revision: 246674

URL: https://gcc.gnu.org/viewcvs?rev=246674=gcc=rev
Log:
2017-04-04  Richard Biener  

PR middle-end/80281
* match.pd (A + (-B) -> A - B): Make sure to preserve unsigned
arithmetic done for the negate or the plus.  Simplify.
(A - (-B) -> A + B): Likewise.
* fold-const.c (split_tree): Make sure to not negate pointers.

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

Added:
trunk/gcc/testsuite/gcc.dg/torture/pr80281.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/fold-const.c
trunk/gcc/match.pd
trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

--- Comment #10 from Richard Biener  ---
So

 /* A + (-B) -> A - B */
 (simplify
  (plus:c (convert1? @0) (convert2? (negate @1)))
  /* Apply STRIP_NOPS on @0 and the negate.  */
  (if (tree_nop_conversion_p (type, TREE_TYPE (@0))
   && tree_nop_conversion_p (type, TREE_TYPE (@1))
   && !TYPE_OVERFLOW_SANITIZED (type))
   (with
{
 tree t1 = type;
 if (TYPE_OVERFLOW_WRAPS (type) != TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
   t1 = TYPE_OVERFLOW_WRAPS (type) ? type : TREE_TYPE (@1);
}
(convert (minus (convert:t1 @0) (convert:t1 @1))

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

--- Comment #9 from Richard Biener  ---
(In reply to Marc Glisse from comment #7)
> (In reply to Richard Biener from comment #6)
> > We need to keep the type of the negation, thus
> 
> If the addition was unsigned, you don't want to turn it into a signed
> subtraction either.

Suggestion?  Apart from disabling the transform when the negate was unsigned
but the plus will be signed for the original pattern?  This set of patterns was
supposed to be safe regardless of overflow... (which they are unless we factor
in conversions...)

So maybe if TYPE_OVERFLOW_WRAPS of the plus and the negate doesn't agree use an
unsigned type throughout?

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

--- Comment #8 from Richard Biener  ---
Hmm, with that associate: endlessly recurses for

(size_t) ivtmp.389_3136 + (size_t) (1 - (sizetype) "[abi:")

(I'm having a Deja-Vu on this).

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[glisse at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

--- Comment #7 from Marc Glisse  ---
(In reply to Richard Biener from comment #6)
> We need to keep the type of the negation, thus

If the addition was unsigned, you don't want to turn it into a signed
subtraction either.

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

--- Comment #6 from Richard Biener  ---
We need to keep the type of the negation, thus

 c + (int)-(unsigned)b  ->  (int)((unsigned) c - (unsigned) b)

 /* A + (-B) -> A - B */
 (simplify
  (plus:c (convert1? @0) (convert2? (negate @1)))
  /* Apply STRIP_NOPS on @0 and the negate.  */
  (if (tree_nop_conversion_p (type, TREE_TYPE (@0))
   && tree_nop_conversion_p (type, TREE_TYPE (@1))
   && !TYPE_OVERFLOW_SANITIZED (type))
   (with { tree t1 = TREE_TYPE (@1); }
(convert (minus (convert:t1 @0) @1)

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

--- Comment #5 from Richard Biener  ---
 /* A + (-B) -> A - B */
 (simplify
  (plus:c (convert1? @0) (convert2? (negate @1)))
  /* Apply STRIP_NOPS on @0 and the negate.  */
  (if (tree_nop_conversion_p (type, TREE_TYPE (@0))
   && tree_nop_conversion_p (type, TREE_TYPE (@1))
   && !TYPE_OVERFLOW_SANITIZED (type))
   (minus (convert @0) (convert @1

(gdb) p debug_generic_expr (captures[1])
(unsigned int) b
$9 = void
(gdb) p debug_generic_expr (captures[0])
c

so that results in c + (int)-(unsigned)b -> c - (int)(unsigned)b.  Similar
code was in tree-ssa-forwprop.c before but that didn't trigger for the testcase
because we first constant propagated.

> gcc-4.8 t.c -O2 -fdisable-tree-ccp1
> ./a.out
Aborted (core dumped)

so probably latent since quite some time.

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

Richard Biener  changed:

   What|Removed |Added

   Priority|P1  |P2
 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |rguenth at gcc dot 
gnu.org

--- Comment #4 from Richard Biener  ---
Mine.

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[law at redhat dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

Jeffrey A. Law  changed:

   What|Removed |Added

   Priority|P3  |P1
 CC||law at redhat dot com

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

Jakub Jelinek  changed:

   What|Removed |Added

 CC||jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek  ---
It has been just latent before that.
Let's change the testcase to:
int
main ()
{
  volatile int a = 0;
  long long b = 2147483648LL;
  int c = a % 2;
  int x = ((int) -b + c) % -2147483647;
  if (x != -1)
__builtin_abort ();
  return 0;
}
so that it works also on ILP32 without triggering UB.
Already in *.original it looks wrong:
int x = (c - (int) b) % 2147483647;
which is incorrect, because originally the negation has been performed in a
wider type where it didn't trigger UB, but in the narrower type it does.
In the original testcase we get -2147483648LL converted to -2147483648 int + 0.
But in what we have in *.original we have 2147483648LL converted to -2147483648
and compute 0 - (-2147483648), which is UB.
So, if we want to transform (int) -b + c into c - (int) b, we actually need to
do the subtraction in unsigned int type and convert back (until we have
separate wrapping signed arithmetic opcodes in GIMPLE if ever).
Thus it should be int x = (int) ((unsigned) c - (unsigned) b) % -2147483647;

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

--- Comment #2 from Marek Polacek  ---
Started with r158965 I think.

[Bug tree-optimization/80281] [5/6/7 Regression] Wrong constant folding

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80281

Marek Polacek  changed:

   What|Removed |Added

   Keywords||wrong-code
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-04-01
 CC||mpolacek at gcc dot gnu.org
   Target Milestone|--- |5.5
Summary|Wrong constant folding  |[5/6/7 Regression] Wrong
   ||constant folding
 Ever confirmed|0   |1

--- Comment #1 from Marek Polacek  ---
Confirmed.  Worked with GCC 4.4.