Bug 81555 - [5/6/7/8 Regression] Wrong code at -O1
Summary: [5/6/7/8 Regression] Wrong code at -O1
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.0
: P2 normal
Target Milestone: 7.2
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks: yarpgen
  Show dependency treegraph
 
Reported: 2017-07-26 00:45 UTC by Dmitry Babokin
Modified: 2021-11-01 23:07 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 4.8.5
Known to fail: 5.4.0, 6.3.0, 7.1.0
Last reconfirmed: 2017-07-26 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Babokin 2017-07-26 00:45:52 UTC
gcc trunk, rev250545, x86_64.

> cat f.cpp
#include <stdio.h>

unsigned int var_1 = 1;
bool var_2 = false;
unsigned int var_3 = 679743406U;
unsigned int var_4 = 3054363510U;
bool var_5 = true;
unsigned char var_6 = 1;

void foo() {
  bool c = var_1 != var_2;
  if (c)
    var_5 = 0;
  if (var_4 & c & (unsigned char)var_3 & c)
    var_6 = 0;
}

int main() {
  foo();
  printf("%d, %d\n", var_5, var_6);
  return 0;
}

> g++ -O0 f.cpp -o out; ./out
0, 1

> g++ -O1 f.cpp -o out; ./out
0, 0
Comment 1 Marc Glisse 2017-07-26 06:34:41 UTC
Same reassoc issue as PR 81556 it seems.
Comment 2 Dmitry Babokin 2017-07-26 06:39:14 UTC
Hmmm, but this one is triggered at -O1, another only at -O2.
Comment 3 Marc Glisse 2017-07-26 06:44:41 UTC
(In reply to Dmitry Babokin from comment #2)
> Hmmm, but this one is triggered at -O1, another only at -O2.

-fno-tree-reassoc should help both.

It is often a combination of optimizations that causes the bug. Reassoc is doing a good transformation, but it leaves wrong information around, which only matters if some other pass (rightfully) takes advantage of that information. Still, it was good to report both, and I expect we may add (a modified version of) both to the testsuite once this is fixed, thanks.
Comment 4 Dmitry Babokin 2017-07-26 06:57:40 UTC
(In reply to Marc Glisse from comment #3)
> -fno-tree-reassoc should help both.
It helps.

> It is often a combination of optimizations that causes the bug. Reassoc is
> doing a good transformation, but it leaves wrong information around, which
> only matters if some other pass (rightfully) takes advantage of that
> information. Still, it was good to report both, and I expect we may add (a
> modified version of) both to the testsuite once this is fixed, thanks.

Agree, two regression tests are better than one.

As a side note, it would be really useful to have a general mechanism for blaming particular optimization by turning off optimizer after certain point (defined through command line switch). Clang and icc both have such mechanism and it's extremely useful. In clang it's "-mllvm -opt-bisect-limit=X". So it enables automated attribution of the bug to particular optimization. Of course it's not 100% precise, but in most of the cases it points to the right place. In other cases it gives good clue where to start the investigation.
Comment 5 Richard Biener 2017-07-26 08:54:44 UTC
Note that with -O2 it also fails with GCC 5 and 6.  GCC 4.8 is fine.
Comment 6 Martin Liška 2017-07-26 11:21:47 UTC
So with -O2 it started to output '0, 0' in r234764.
For -O1 it started to output '0, 0' in r236338.
Comment 7 Jakub Jelinek 2017-07-27 08:49:53 UTC
Author: jakub
Date: Thu Jul 27 08:49:16 2017
New Revision: 250609

URL: https://gcc.gnu.org/viewcvs?rev=250609&root=gcc&view=rev
Log:
	PR tree-optimization/81555
	PR tree-optimization/81556
	* tree-ssa-reassoc.c (rewrite_expr_tree): Add NEXT_CHANGED argument,
	if true, force CHANGED for the recursive invocation.
	(reassociate_bb): Remember original length of ops array, pass
	len != orig_len as NEXT_CHANGED in rewrite_expr_tree call.

	* gcc.c-torture/execute/pr81555.c: New test.
	* gcc.c-torture/execute/pr81556.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr81555.c
    trunk/gcc/testsuite/gcc.c-torture/execute/pr81556.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-reassoc.c
Comment 8 Jakub Jelinek 2017-07-27 09:06:41 UTC
Author: jakub
Date: Thu Jul 27 09:06:08 2017
New Revision: 250610

URL: https://gcc.gnu.org/viewcvs?rev=250610&root=gcc&view=rev
Log:
	PR tree-optimization/81555
	PR tree-optimization/81556
	* tree-ssa-reassoc.c (rewrite_expr_tree): Add NEXT_CHANGED argument,
	if true, force CHANGED for the recursive invocation.
	(reassociate_bb): Remember original length of ops array, pass
	len != orig_len as NEXT_CHANGED in rewrite_expr_tree call.

	* gcc.c-torture/execute/pr81555.c: New test.
	* gcc.c-torture/execute/pr81556.c: New test.

Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.c-torture/execute/pr81555.c
    branches/gcc-7-branch/gcc/testsuite/gcc.c-torture/execute/pr81556.c
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
    branches/gcc-7-branch/gcc/tree-ssa-reassoc.c
Comment 9 Jakub Jelinek 2017-07-27 09:33:10 UTC
Author: jakub
Date: Thu Jul 27 09:32:33 2017
New Revision: 250612

URL: https://gcc.gnu.org/viewcvs?rev=250612&root=gcc&view=rev
Log:
	PR tree-optimization/81555
	PR tree-optimization/81556
	* tree-ssa-reassoc.c (rewrite_expr_tree): Add NEXT_CHANGED argument,
	if true, force CHANGED for the recursive invocation.
	(reassociate_bb): Remember original length of ops array, pass
	len != orig_len as NEXT_CHANGED in rewrite_expr_tree call.

	* gcc.c-torture/execute/pr81555.c: New test.
	* gcc.c-torture/execute/pr81556.c: New test.

Added:
    branches/gcc-6-branch/gcc/testsuite/gcc.c-torture/execute/pr81555.c
    branches/gcc-6-branch/gcc/testsuite/gcc.c-torture/execute/pr81556.c
Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/testsuite/ChangeLog
    branches/gcc-6-branch/gcc/tree-ssa-reassoc.c
Comment 10 Jakub Jelinek 2017-07-27 09:43:34 UTC
Author: jakub
Date: Thu Jul 27 09:43:01 2017
New Revision: 250616

URL: https://gcc.gnu.org/viewcvs?rev=250616&root=gcc&view=rev
Log:
	PR tree-optimization/81555
	PR tree-optimization/81556
	* tree-ssa-reassoc.c (rewrite_expr_tree): Add NEXT_CHANGED argument,
	if true, force CHANGED for the recursive invocation.
	(reassociate_bb): Remember original length of ops array, pass
	len != orig_len as NEXT_CHANGED in rewrite_expr_tree call.

	* gcc.c-torture/execute/pr81555.c: New test.
	* gcc.c-torture/execute/pr81556.c: New test.

Added:
    branches/gcc-5-branch/gcc/testsuite/gcc.c-torture/execute/pr81555.c
    branches/gcc-5-branch/gcc/testsuite/gcc.c-torture/execute/pr81556.c
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
    branches/gcc-5-branch/gcc/tree-ssa-reassoc.c
Comment 11 Jakub Jelinek 2017-07-28 07:17:03 UTC
Fixed.
Comment 12 Aldy Hernandez 2017-09-13 15:51:24 UTC
Author: aldyh
Date: Wed Sep 13 15:50:53 2017
New Revision: 252110

URL: https://gcc.gnu.org/viewcvs?rev=252110&root=gcc&view=rev
Log:
	PR tree-optimization/81555
	PR tree-optimization/81556
	* tree-ssa-reassoc.c (rewrite_expr_tree): Add NEXT_CHANGED argument,
	if true, force CHANGED for the recursive invocation.
	(reassociate_bb): Remember original length of ops array, pass
	len != orig_len as NEXT_CHANGED in rewrite_expr_tree call.

	* gcc.c-torture/execute/pr81555.c: New test.
	* gcc.c-torture/execute/pr81556.c: New test.

Added:
    branches/range-gen2/gcc/testsuite/gcc.c-torture/execute/pr81555.c
    branches/range-gen2/gcc/testsuite/gcc.c-torture/execute/pr81556.c
Modified:
    branches/range-gen2/gcc/ChangeLog
    branches/range-gen2/gcc/testsuite/ChangeLog
    branches/range-gen2/gcc/tree-ssa-reassoc.c