Bug 46970 - [4.3/4.4/4.5/4.6 Regression] wrong code with -Os -ftree-loop-linear
[4.3/4.4/4.5/4.6 Regression] wrong code with -Os -ftree-loop-linear
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: tree-optimization
4.6.0
: P2 normal
: 4.3.6
Assigned To: Sebastian Pop
: wrong-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-15 20:11 UTC by Zdenek Sojka
Modified: 2011-02-02 17:49 UTC (History)
4 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu
Build:
Known to work: 4.0.4
Known to fail: 4.1.2, 4.2.4, 4.3.5, 4.4.6, 4.5.2, 4.6.0
Last reconfirmed: 2010-12-16 00:45:57


Attachments
reduced testcase (from gcc.dg/pr29581-3.c) (281 bytes, text/plain)
2010-12-15 20:11 UTC, Zdenek Sojka
Details
gcc46-pr46970.patch (4.21 KB, patch)
2010-12-17 17:14 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zdenek Sojka 2010-12-15 20:11:42 UTC
Created attachment 22774 [details]
reduced testcase (from gcc.dg/pr29581-3.c)

Output:
$ gcc -Os -ftree-loop-linear pr46970.c
or
$ gcc -O -fno-tree-ch -ftree-loop-linear pr46970.c
(-Os implies -fno-tree-ch)

$ ./a.out 
Aborted

Tested versions:
r167809 - fail
4.5.1 - fail
4.4.5 - fail
4.3.5 - fail
4.2.4 - ICE
4.1.2 - ICE
4.0.4 - OK
Comment 1 H.J. Lu 2010-12-16 00:45:57 UTC
Revision 131435:

http://gcc.gnu.org/ml/gcc-cvs/2008-01/msg00195.html

fixed ICE and generated wrong code.
Comment 2 H.J. Lu 2010-12-16 00:49:40 UTC
It is related to PR 34017.
Comment 3 H.J. Lu 2010-12-16 00:51:24 UTC
Patch was posted at

http://gcc.gnu.org/ml/gcc-patches/2008-01/msg00371.html
Comment 4 Jakub Jelinek 2010-12-17 16:17:39 UTC
I think the bug is in that gcc_loop_to_lambda_loop doesn't record whether the SSA_NAME compared in the exit condition was the induction var or incremented induction var.
E.g. on ltrans-3.c testcase we have
<bb 5>:
  # j_25 = PHI <j_15(4), 0(9)>
...
  j_15 = j_25 + 1;
  if (N.2_3 > j_15)
    goto <bb 4>;
  else
    goto <bb 6>;
and thus the exit condition tests the incremented iv (gcc_loop_to_lambda_loop btw. doesn't bother to check whether the def stmt of the rhs of exit condition here actually increments by step, and doesn't bother to check that step fits into int (so I guess step like 0x100000001ULL would cause trouble).
It just records lower bound 0, upper bound N.2_3 - 1 here and step 1.
But with -Os -ftree-loop-linear on this testcase we have before ltrans:
  j_12 = j_2 + 1;

<bb 4>:
  # j_2 = PHI <0(7), j_12(3)>
  if (j_2 < n_5(D))
    goto <bb 3>;
  else
    goto <bb 5>;
Here similarly it records lower bound 0, upper bound n_5 - 1 and step 1.
But there is an important difference in between the two.
In the former case we correctly use
  gimple_cond_set_condition (exitcond, testtype, newupperbound, ivvarinced);
but we use it in the second case too, which is wrong.
Comment 5 Jakub Jelinek 2010-12-17 17:14:01 UTC
Created attachment 22801 [details]
gcc46-pr46970.patch

Untested patch that fixes this and keeps ltrans-3.c working.

I don't feel very good about the +-1 * stepint extra adjustments the code did and still does though, I'm afraid there could be problems with that if there is an overflow, but it would be much more work to adjust lambda-code.c so that it deals with this properly.
Comment 6 Sebastian Pop 2011-01-18 20:54:55 UTC
Author: spop
Date: Tue Jan 18 20:54:52 2011
New Revision: 168967

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168967
Log:
Add testcase for PR46970.

2011-01-18  Sebastian Pop  <sebastian.pop@amd.com>

	PR tree-optimization/46970
	* gcc.dg/graphite/pr46970.c: New.

Added:
    branches/graphite/gcc/testsuite/gcc.dg/graphite/pr46970.c
Modified:
    branches/graphite/gcc/ChangeLog.graphite
Comment 7 Sebastian Pop 2011-01-18 20:56:30 UTC
Fixed on the graphite branch.
Comment 8 Sebastian Pop 2011-01-25 21:25:35 UTC
Author: spop
Date: Tue Jan 25 21:25:24 2011
New Revision: 169257

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169257
Log:
Add testcase for PR46970.

2011-01-18  Sebastian Pop  <sebastian.pop@amd.com>

	PR tree-optimization/46970
	* gcc.dg/graphite/pr46970.c: New.

Added:
    trunk/gcc/testsuite/gcc.dg/graphite/pr46970.c
Modified:
    trunk/gcc/testsuite/ChangeLog
Comment 9 Sebastian Pop 2011-01-25 21:28:37 UTC
Fixed.
Comment 10 Diego Novillo 2011-02-02 17:49:01 UTC
Author: dnovillo
Date: Wed Feb  2 17:48:51 2011
New Revision: 169596

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169596
Log:
Add testcase for PR46970.

2011-01-18  Sebastian Pop  <sebastian.pop@amd.com>

	PR tree-optimization/46970
	* gcc.dg/graphite/pr46970.c: New.

Added:
    branches/google/integration/gcc/testsuite/gcc.dg/graphite/pr46970.c
Modified:
    branches/google/integration/gcc/testsuite/ChangeLog