public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-2048] Fix 101256: Wrong code due to range incorrect from PHI-OPT
@ 2021-07-06  7:34 Andrew Pinski
  0 siblings, 0 replies; only message in thread
From: Andrew Pinski @ 2021-07-06  7:34 UTC (permalink / raw)
  To: gcc-cvs

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.

Diff:
---
 gcc/dbgcnt.def                          |  1 +
 gcc/testsuite/g++.dg/torture/pr101256.C | 28 ++++++++++++++++++++++++++++
 gcc/tree-ssa-phiopt.c                   | 16 ++++++++++------
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def
index 93e7b4fd30e..2345899ba68 100644
--- a/gcc/dbgcnt.def
+++ b/gcc/dbgcnt.def
@@ -183,6 +183,7 @@ DEBUG_COUNTER (lim)
 DEBUG_COUNTER (local_alloc_for_sched)
 DEBUG_COUNTER (match)
 DEBUG_COUNTER (merged_ipa_icf)
+DEBUG_COUNTER (phiopt_edge_range)
 DEBUG_COUNTER (postreload_cse)
 DEBUG_COUNTER (pre)
 DEBUG_COUNTER (pre_insn)
diff --git a/gcc/testsuite/g++.dg/torture/pr101256.C b/gcc/testsuite/g++.dg/torture/pr101256.C
new file mode 100644
index 00000000000..973a8b4caf3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr101256.C
@@ -0,0 +1,28 @@
+// { dg-do run }
+
+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 (int 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;
+}
diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index ab63bf699e3..8b60ee81082 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "internal-fn.h"
 #include "gimple-range.h"
 #include "gimple-match.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 *,
@@ -390,7 +391,7 @@ replace_phi_edge_with_variable (basic_block cond_block,
   gimple_stmt_iterator gsi;
   tree phi_result = PHI_RESULT (phi);
 
-  /* Duplicate range info if we're the only things setting the target PHI.
+  /* Duplicate range info if they are the only things setting the target PHI.
      This is needed as later on, the new_tree will be replacing
      The assignement of the PHI.
      For an example:
@@ -398,19 +399,22 @@ replace_phi_edge_with_variable (basic_block cond_block,
      _4 = min<a_1, 255>
      goto bb2
 
-     range<-INF,255>
+     # RANGE [-INF, 255]
      a_3 = PHI<_4(1)>
      bb3:
 
      use(a_3)
-     And _4 gets prograted into the use of a_3 and losing the range info.
-     This can't be done for more than 2 incoming edges as the progration
-     won't happen.  */
+     And _4 gets propagated into the use of a_3 and losing the range info.
+     This can't be done for more than 2 incoming edges as the propagation
+     won't happen.
+     The new_tree needs to be defined in the same basic block as the conditional.  */
   if (TREE_CODE (new_tree) == SSA_NAME
       && 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)
+      && gimple_bb (SSA_NAME_DEF_STMT (new_tree)) == cond_block
+      && dbg_cnt (phiopt_edge_range))
     duplicate_ssa_name_range_info (new_tree,
 				   SSA_NAME_RANGE_TYPE (phi_result),
 				   SSA_NAME_RANGE_INFO (phi_result));


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-07-06  7:34 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-06  7:34 [gcc r12-2048] Fix 101256: Wrong code due to range incorrect from PHI-OPT Andrew Pinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).