Bug 81403 - [8 Regression] wrong code at -O3
Summary: [8 Regression] wrong code at -O3
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 8.0
: P1 normal
Target Milestone: 8.0
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on:
Blocks: yarpgen
  Show dependency treegraph
 
Reported: 2017-07-11 22:39 UTC by Dmitry Babokin
Modified: 2021-11-01 23:07 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-07-12 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-11 22:39:47 UTC
gcc rev250140, x86_64.

> cat f.cpp
#include <iostream>

short var_9 = 19581;
unsigned char var_33 = 21;
long int var_55 = 286697804684061197L;
long int var_59 = -1962393262513510540L;
long int var_71 = 4731868609112929952L;
long int var_773 = -4784633456247777769L;
short var_776 = 5894;
long int var_1321 = 7573221950916697355L;
unsigned char uc = 217;

void foo() {
  if (var_55)
    var_71 = 0;
  if (var_9 != ~(0 < uc))
    var_773 = 0;
  else
    var_776 = 1 / ~var_9 * -1;
  if (var_33)
    var_59 = ~var_9 & 10393;
  var_1321 = ~var_9;
}

int main() {
  foo();
  std::cout << "var_59 = " << var_59 << " (8320 expected)\n";

  return 0;
}

> g++ -w -O3 -o out f.cpp; ./out
var_59 = -19582 (8320 expected)
> g++ -w -O2 -o out f.cpp; ./out
var_59 = 8320 (8320 expected)
> g++ -w -O0 -o out f.cpp; ./out
var_59 = 8320 (8320 expected)
Comment 1 Marc Glisse 2017-07-12 06:58:51 UTC
PRE losing "& 10393" at -O3 but not -O2 (the previous dumps are identical)

@@ -611,6 +639,7 @@
 ;;                6 [100.0%]  (FALLTHRU,EXECUTABLE)
   # .MEM_21 = PHI <.MEM_26(5), .MEM_25(6)>
   # prephitmp_34 = PHI <_30(5), _30(6)>
+  # prephitmp_35 = PHI <_30(5), _30(6)>
   # VUSE <.MEM_21>
   var_33.4_11 = var_33D.35372;
   if (var_33.4_11 != 0)
@@ -624,9 +653,7 @@
 ;;    prev block 7, next block 9, flags: (NEW, REACHABLE, VISITED)
 ;;    pred:       7 [54.0%]  (TRUE_VALUE,EXECUTABLE)
   # RANGE [0, 10393] NONZERO 10393
-  _29 = prephitmp_34 & 10393;
-  # RANGE [0, 10393] NONZERO 10393
-  _15 = (long intD.12) _29;
+  _15 = (long intD.12) prephitmp_35;
Comment 2 Martin Liška 2017-07-12 07:11:02 UTC
Confirmed, started with r247596.
Comment 3 Marc Glisse 2017-07-12 07:14:51 UTC
/* x & C -> x if we know that x & ~C == 0.  */

Not clear where it is getting the bogus range/nonzero information from, I thought we had fixed all the places reusing SSA_NAMEs with stale information.
Comment 4 Richard Biener 2017-07-17 11:13:43 UTC
Mine :/
Comment 5 Richard Biener 2017-07-17 14:31:22 UTC
So this is during partial-PRE insertion where after PRE insertion of

Found partial redundancy for expression {bit_not_expr,_13} (0020)
Inserted _33 = ~_3;
 in predecessor 5 (0014)
Created phi prephitmp_34 = PHI <_33(5), _8(6)>
 in block 7 (0020)

we value-replace 0020 by prephitmp_34 and thus during partial-PRE phi-translation
find, when translating _14 & 10393, _8 for _14 (0020) to be translated across
the edge exposing the range.

On the other edge we get from translating _14 & 10393 all the way to translating
var_9.5_12 = var_9 as operand which leads us to var_9.1_2 = var_9 and ultimatively
to _8 via "simplification" of ~_3 which has a leader of _33 (fine IMHO), but
unfortunately _33 has a value number of _8.

So PRE doing any insertion of a lexically equivalent expression will necessarily
end up with a SCCVN value-number that might have range info that is not valid
there.  Thus:

            tree result = vn_nary_op_lookup_pieces (newnary->length,
                                                    newnary->opcode,
                                                    newnary->type,
                                                    &newnary->op[0],
                                                    &nary);
            if (result && is_gimple_min_invariant (result))
              return get_or_alloc_expr_for_constant (result);

            expr = pre_expr_pool.allocate ();
            expr->kind = NARY;
            expr->id = 0;
            if (nary)
              {
                PRE_EXPR_NARY (expr) = nary;
                new_val_id = nary->value_id;
                get_or_alloc_expression_id (expr);
              }

needs adjustment to push away range-info.
Comment 6 Richard Biener 2017-07-17 14:35:37 UTC
Kind-of a duplicate of PR80620 as well.  Testing a patch.
Comment 7 Richard Biener 2017-07-18 07:36:14 UTC
Author: rguenth
Date: Tue Jul 18 07:35:40 2017
New Revision: 250297

URL: https://gcc.gnu.org/viewcvs?rev=250297&root=gcc&view=rev
Log:
2017-07-18  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/80620
	PR tree-optimization/81403
	* tree-ssa-pre.c (phi_translate_1): Clear range and points-to
	info when re-using a VN table entry.

	* gcc.dg/torture/pr80620.c: New testcase.
	* gcc.dg/torture/pr81403.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr80620.c
    trunk/gcc/testsuite/gcc.dg/torture/pr81403.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-pre.c
Comment 8 Richard Biener 2017-07-18 07:36:31 UTC
Fixed.
Comment 9 Richard Biener 2017-11-23 08:33:10 UTC
Author: rguenth
Date: Thu Nov 23 08:30:41 2017
New Revision: 255092

URL: https://gcc.gnu.org/viewcvs?rev=255092&root=gcc&view=rev
Log:
2017-11-23  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/81403
	* tree-ssa-pre.c (get_representative_for): Add parameter specifying
	a block we need a leader relative to.
	(phi_translate_1): For nary processing require a leader from
	get_representative_for given we run expression simplification
	using match-and-simplify.  Remove previous fix.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-pre.c