[Bug tree-optimization/101256] New: [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[Bug tree-optimization/101256] New: [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[marxin at gcc dot gnu.org]

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

            Bug ID: 101256
           Summary: [12 Regression] Wrong code with -O3 since
                    r12-1841-g9fe9c45ae33a2df7
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: pinskia at gcc dot gnu.org
  Target Milestone: ---

Created attachment 51081
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51081&action=edit
test-case

It's a yarpgen test case and it's not easily reducible.

However, with the following patch:


diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def
index 93e7b4fd30e..6186b5533ca 100644
--- a/gcc/dbgcnt.def
+++ b/gcc/dbgcnt.def
@@ -158,6 +158,7 @@ DEBUG_COUNTER (dom_unreachable_edges)
 DEBUG_COUNTER (dse)
 DEBUG_COUNTER (dse1)
 DEBUG_COUNTER (dse2)
+DEBUG_COUNTER (edge_range)
 DEBUG_COUNTER (gcse2_delete)
 DEBUG_COUNTER (gimple_unroll)
 DEBUG_COUNTER (global_alloc_at_func)
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index ab12e85569d..21741504038 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -50,6 +50,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "gimple-fold.h"
 #include "internal-fn.h"
 #include "gimple-range.h"
+#include "dbgcnt.h"

 static unsigned int tree_ssa_phiopt_worker (bool, bool, bool);
 static bool two_value_replacement (basic_block, basic_block, edge, gphi *,
@@ -413,7 +414,8 @@ replace_phi_edge_with_variable (basic_block cond_block,
       && EDGE_COUNT (gimple_bb (phi)->preds) == 2
       && INTEGRAL_TYPE_P (TREE_TYPE (phi_result))
       && !SSA_NAME_RANGE_INFO (new_tree)
-      && SSA_NAME_RANGE_INFO (phi_result))
+      && SSA_NAME_RANGE_INFO (phi_result)
+      && dbg_cnt(edge_range))
     duplicate_ssa_name_range_info (new_tree,
                                   SSA_NAME_RANGE_TYPE (phi_result),
                                   SSA_NAME_RANGE_INFO (phi_result));

One can reduce it to:

$ g++ driver.cpp -c && g++ func.cpp -O3 -c -fdbg-cnt=edge_range:52-52 && gcc
func.o driver.o && ./a.out
***dbgcnt: lower limit 52 reached for edge_range.***
***dbgcnt: upper limit 52 reached for edge_range.***
a.out: driver.cpp:3609: void checksum(): Assertion `var_303 == (unsigned
char)240' failed.
Aborted (core dumped)

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |11.1.0
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-06-29
      Known to fail|                            |12.0
     Ever confirmed|0                           |1

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Mine.

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a fix but I don't have a testcase really.
What is happening is value_replacement is happening but since the ssa_name that
is being used for replace_phi_edge_with_variable is not newly defined, we are
adding the range info from something which was conditionally defined.

To be able to make sure we are doing the range for a newly defined name, we
need to pass the sequence of statements that get moved/added before
conditional.

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 51087
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51087&action=edit
Patch which I am testing

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[marxin at gcc dot gnu.org]

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

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
I managed reducing a test-case:

$ cat driver.cpp 
#include <stdio.h>
#include <cassert>

template<class T> 
const T& max(const T& a, const T& b)
{
    return (a < b) ? b : a;
}

signed char var_5 = -128;
unsigned int var_11 = 2144479212U;
unsigned long long int arr [22];

void
__attribute__((noipa))
test(signed char var_5, unsigned var_11) {
  for (short i_61 = 0; i_61 < var_5 + 149; i_61 += 10000)
    arr[i_61] = max((signed char)0, var_5) ? max((signed char)1, var_5) :
var_11;
}

int main() {
  for (size_t i_0 = 0; i_0 < 22; ++i_0) 
      arr [i_0] = 11834725929543695741ULL;

    test(var_5, var_11);
    assert(arr [0] == 2144479212ULL || arr [0]  == 11834725929543695741ULL);
    __builtin_printf ("OK\n");
}

$ g++ driver.cpp -O2 && ./a.out 
a.out: driver.cpp:26: int main(): Assertion `arr [0] == 2144479212ULL || arr
[0] == 11834725929543695741ULL' failed.
Aborted (core dumped)
$ g++ driver.cpp && ./a.out 
OK

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[marxin at gcc dot gnu.org]

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

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
And I can confirm the patch candidate fixes that.

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
And here is a self-contained testcase:

template<class T> 
const T& max(const T& a, const T& b)
{
    return (a < b) ? b : a;
}

signed char var_5 = -128;
unsigned int var_11 = 2144479212U;
unsigned long long int arr [22];

void
__attribute__((noipa))
test(signed char var_5, unsigned var_11) {
  for (short i_61 = 0; i_61 < var_5 + 149; i_61 += 10000)
    arr[i_61] = max((signed char)0, var_5) ? max((signed char)1, var_5) :
var_11;
}

int main() {
  for (size_t i_0 = 0; i_0 < 22; ++i_0) 
      arr [i_0] = 11834725929543695741ULL;

  test(var_5, var_11);
  if (arr [0] != 2144479212ULL && arr [0] != 11834725929543695741ULL)
    __builtin_abort ();
  return 0;
}

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|https://gcc.gnu.org/piperma |https://gcc.gnu.org/piperma
                   |il/gcc-patches/2021-July/57 |il/gcc-patches/2021-July/57
                   |4412.html                   |4490.html

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Updated patch:
https://gcc.gnu.org/pipermail/gcc-patches/2021-July/574490.html

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:51abfb6a893c87dbf84a33009b6cd6dbd25d66f1

commit r12-2048-g51abfb6a893c87dbf84a33009b6cd6dbd25d66f1
Author: Andrew Pinski <apinski@marvell.com>
Date:   Tue Jun 29 14:30:34 2021 -0700

    Fix 101256: Wrong code due to range incorrect from PHI-OPT

    So the problem here is that replace_phi_edge_with_variable
    will copy range information to a already (not newly) defined
    ssa name.  This causes wrong code later on.
    This fixes the problem by require the new ssa name to
    be defined in the same bb as the conditional that is
    about to be deleted.

    OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

    Changes from v1:
    * this is a simplification of what was trying to be done before.

    gcc/ChangeLog:

            PR tree-optimization/101256
            * dbgcnt.def (phiopt_edge_range): New counter.
            * tree-ssa-phiopt.c (replace_phi_edge_with_variable):
            Check to make sure the new name is defined in the same
            bb as the conditional before duplicating range info.
            Also add debug counter.

    gcc/testsuite/ChangeLog:

            PR tree-optimization/101256
            * g++.dg/torture/pr101256.C: New test.

[Bug tree-optimization/101256] [12 Regression] Wrong code with -O3 since r12-1841-g9fe9c45ae33a2df7

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.