On 6/23/2021 4:19 PM, apinski--- via Gcc-patches wrote:
From: Andrew Pinski <apin...@marvell.com>

Since match_simplify_replacement uses gimple_simplify, there is a new
ssa name created sometimes and then we go and replace the phi edge with
this new ssa name, the range information on the phi is lost.
Placing this in replace_phi_edge_with_variable is the best option instead
of doing it in each time replace_phi_edge_with_variable is called which is
what is done today.

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

gcc/ChangeLog:

        * tree-ssa-phiopt.c (replace_phi_edge_with_variable): Duplicate range
        info if we're the only things setting the target PHI.
        (value_replacement): Don't duplicate range here.
        (minmax_replacement): Likewise.
---
  gcc/tree-ssa-phiopt.c | 35 ++++++++++++++++++-----------------
  1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
index 24cbce9955a..147397ad82c 100644
--- a/gcc/tree-ssa-phiopt.c
+++ b/gcc/tree-ssa-phiopt.c
@@ -391,6 +391,24 @@ replace_phi_edge_with_variable (basic_block cond_block,
    basic_block bb = gimple_bb (phi);
    basic_block block_to_remove;
    gimple_stmt_iterator gsi;
+  tree phi_result = PHI_RESULT (phi);
+
+  /* Duplicate range info as needed.  */
+  if (TREE_CODE (new_tree) == SSA_NAME
+      && EDGE_COUNT (gimple_bb (phi)->preds) == 2
+      && INTEGRAL_TYPE_P (TREE_TYPE (phi_result)))
+    {
+      if (!SSA_NAME_RANGE_INFO (new_tree)
+         && SSA_NAME_RANGE_INFO (phi_result))
+       duplicate_ssa_name_range_info (new_tree,
+                                      SSA_NAME_RANGE_TYPE (phi_result),
+                                      SSA_NAME_RANGE_INFO (phi_result));
+      if (SSA_NAME_RANGE_INFO (new_tree)
+         && !SSA_NAME_RANGE_INFO (phi_result))
+       duplicate_ssa_name_range_info (phi_result,
+                                      SSA_NAME_RANGE_TYPE (new_tree),
+                                      SSA_NAME_RANGE_INFO (new_tree));
+    }
I'm not sure you need the EDGE_COUNT test here.

I worry that copying the range info from the argument to the PHI result isn't right.  It was OK for the ABS replacement, but I'm not sure that's true in general.

Jeff


/* Change the PHI argument to new. */
    SET_USE (PHI_ARG_DEF_PTR (phi, e->dest_idx), new_tree);
@@ -1385,16 +1403,6 @@ value_replacement (basic_block cond_bb, basic_block 
middle_bb,
           <bb 4>:
           # u_3 = PHI <u_6(3), 4294967295(2)>  */
        reset_flow_sensitive_info (lhs);
-      if (INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
-       {
-         /* If available, we can use VR of phi result at least.  */
-         tree phires = gimple_phi_result (phi);
-         struct range_info_def *phires_range_info
-           = SSA_NAME_RANGE_INFO (phires);
-         if (phires_range_info)
-           duplicate_ssa_name_range_info (lhs, SSA_NAME_RANGE_TYPE (phires),
-                                          phires_range_info);
-       }
        gimple_stmt_iterator gsi_from;
        for (int i = prep_cnt - 1; i >= 0; --i)
        {
@@ -1794,13 +1802,6 @@ minmax_replacement (basic_block cond_bb, basic_block 
middle_bb,
    gimple_seq stmts = NULL;
    tree phi_result = PHI_RESULT (phi);
    result = gimple_build (&stmts, minmax, TREE_TYPE (phi_result), arg0, arg1);
-  /* Duplicate range info if we're the only things setting the target PHI.  */
-  if (!gimple_seq_empty_p (stmts)
-      && EDGE_COUNT (gimple_bb (phi)->preds) == 2
-      && !POINTER_TYPE_P (TREE_TYPE (phi_result))
-      && SSA_NAME_RANGE_INFO (phi_result))
-    duplicate_ssa_name_range_info (result, SSA_NAME_RANGE_TYPE (phi_result),
-                                  SSA_NAME_RANGE_INFO (phi_result));
gsi = gsi_last_bb (cond_bb);
    gsi_insert_seq_before (&gsi, stmts, GSI_NEW_STMT);

Reply via email to