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

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 10 Dec 2024, pinskia at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117965
> 
> --- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> Created attachment 59825
>   --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=59825&action=edit
> Fix
> 
> Note this includes debugging still and no testcases. And has only done some
> quick testing on the testcases.

Another alternative is to not fail when !phivn_valid_p but to insert
a dereference on the edge (IIRC you had a patch adjusting phiprop to
consider not just ADDR_EXPRs).  I'd still require at least one
ADDR_EXPR (or chained value) for the transform to be profitable
(or at most one inserted deref).

Note the simplest fix, not using bb_has_eh_pred or so is to simply
allow phivn[SSA_NAME_VERSION (name)].vuse == gimple_vuse of the
original deref.  We find that only after checking all PHI args.
We could possibly delay this VUSE match fallback to that phase.
So maybe it's not the "simplest" fix.

bb_has_eh_pred requires the VUSE use to be in the EH receiver directly,
not allowing for that receiver to flow to the VUSE by other means.

I also think that

  FOR_EACH_IMM_USE_STMT (use_stmt, ui2, vuse)
    {
      /* If BB does not dominate a VDEF, the value is invalid.  */
      if ((gimple_vdef (use_stmt) != NULL_TREE 
           || gimple_code (use_stmt) == GIMPLE_PHI)
          && !dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), bb))
        {   
          ok = false;
          break;

is the wrong way around - once we find a VUSE stmt that dominates BB
the value is OK, not the other way around (find one that doesn't and
it's invalid).  A PHI doesn't work there though (it works only at
the relevant PHI arg defs source block, but there dominance can't
work).

So

diff --git a/gcc/tree-ssa-phiprop.cc b/gcc/tree-ssa-phiprop.cc
index 2a1cdae46d2..5a3012107f9 100644
--- a/gcc/tree-ssa-phiprop.cc
+++ b/gcc/tree-ssa-phiprop.cc
@@ -108,24 +108,20 @@ phivn_valid_p (struct phiprop_d *phivn, tree name, 
basic_block bb)
   tree vuse = phivn[SSA_NAME_VERSION (name)].vuse;
   gimple *use_stmt;
   imm_use_iterator ui2;
-  bool ok = true;

   /* The def stmts of the virtual uses need to be dominated by bb.  */
   gcc_assert (vuse != NULL_TREE);

   FOR_EACH_IMM_USE_STMT (use_stmt, ui2, vuse)
     {
-      /* If BB does not dominate a VDEF, the value is invalid.  */
-      if ((gimple_vdef (use_stmt) != NULL_TREE
-          || gimple_code (use_stmt) == GIMPLE_PHI)
-         && !dominated_by_p (CDI_DOMINATORS, gimple_bb (use_stmt), bb))
-       {
-         ok = false;
-         break;
-       }
+      /* If the VUSE stmt dominates BB then the value is valid.  */
+      if (gimple_vdef (use_stmt) != NULL_TREE
+         && gimple_code (use_stmt) != GIMPLE_PHI
+         && dominated_by_p (CDI_DOMINATORS, bb, gimple_bb (use_stmt)))
+       return true;
     }

-  return ok;
+  return false;
 }

 /* Insert a new phi node for the dereference of PHI at basic_block

Reply via email to