public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Richard Biener <rguenth@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc r13-6113] Fix wrong-code issue in VN
Date: Fri, 17 Feb 2023 11:31:58 +0000 (GMT)	[thread overview]
Message-ID: <20230217113158.2AC4A382E69E@sourceware.org> (raw)

https://gcc.gnu.org/g:417e95263ca4d7a6623783ad664cf6305d8d3fad

commit r13-6113-g417e95263ca4d7a6623783ad664cf6305d8d3fad
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Feb 17 09:30:49 2023 +0100

    Fix wrong-code issue in VN
    
    The following fixes the wrong removed dead store discovered on the
    PR108657 testcase when the reported DSE issue is not fixed.
    The issue is we were using ssa_undefined_value_p on virtual operands
    which returns a result based on the real definition of the definition
    statement.  That doesn't make sense so this patch guards the calls
    properly and makes sure nobody else does the same mistake.
    
            * tree-ssa.cc (ssa_undefined_value_p): Assert we are not
            called on virtual operands.
            * tree-ssa-sccvn.cc (vn_phi_lookup): Guard
            ssa_undefined_value_p calls.
            (vn_phi_insert): Likewise.
            (set_ssa_val_to): Likewise.
            (visit_phi): Avoid extra work with equivalences for
            virtual operand PHIs.

Diff:
---
 gcc/tree-ssa-sccvn.cc | 20 +++++++++++++++-----
 gcc/tree-ssa.cc       |  2 ++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc
index 8ee77fd2b78..d5b081a309f 100644
--- a/gcc/tree-ssa-sccvn.cc
+++ b/gcc/tree-ssa-sccvn.cc
@@ -4824,7 +4824,8 @@ vn_phi_lookup (gimple *phi, bool backedges_varying_p)
       if (TREE_CODE (def) == SSA_NAME
 	  && (!backedges_varying_p || !(e->flags & EDGE_DFS_BACK)))
 	{
-	  if (ssa_undefined_value_p (def, false))
+	  if (!virtual_operand_p (def)
+	      && ssa_undefined_value_p (def, false))
 	    def = VN_TOP;
 	  else
 	    def = SSA_VAL (def);
@@ -4877,7 +4878,8 @@ vn_phi_insert (gimple *phi, tree result, bool backedges_varying_p)
       if (TREE_CODE (def) == SSA_NAME
 	  && (!backedges_varying_p || !(e->flags & EDGE_DFS_BACK)))
 	{
-	  if (ssa_undefined_value_p (def, false))
+	  if (!virtual_operand_p (def)
+	      && ssa_undefined_value_p (def, false))
 	    def = VN_TOP;
 	  else
 	    def = SSA_VAL (def);
@@ -5059,6 +5061,7 @@ set_ssa_val_to (tree from, tree to)
 	}
       curr_invariant = is_gimple_min_invariant (currval);
       curr_undefined = (TREE_CODE (currval) == SSA_NAME
+			&& !virtual_operand_p (currval)
 			&& ssa_undefined_value_p (currval, false));
       if (currval != VN_TOP
 	  && !curr_invariant
@@ -5081,6 +5084,7 @@ set_ssa_val_to (tree from, tree to)
       else if (currval != VN_TOP
 	       && !curr_undefined
 	       && TREE_CODE (to) == SSA_NAME
+	       && !virtual_operand_p (to)
 	       && ssa_undefined_value_p (to, false))
 	{
 	  if (dump_file && (dump_flags & TDF_DETAILS))
@@ -5116,6 +5120,7 @@ set_and_exit:
          PR82320 for a testcase were we'd otherwise not terminate iteration.  */
       && !(curr_undefined
 	   && TREE_CODE (to) == SSA_NAME
+	   && !virtual_operand_p (to)
 	   && ssa_undefined_value_p (to, false))
       /* ???  For addresses involving volatile objects or types operand_equal_p
          does not reliably detect ADDR_EXPRs as equal.  We know we are only
@@ -5880,7 +5885,14 @@ visit_phi (gimple *phi, bool *inserted, bool backedges_varying_p)
 	    sameval = def;
 	    sameval_e = e;
 	  }
-	else if (!expressions_equal_p (def, sameval))
+	else if (expressions_equal_p (def, sameval))
+	  sameval_e = NULL;
+	else if (virtual_operand_p (def))
+	  {
+	    sameval = NULL_TREE;
+	    break;
+	  }
+	else
 	  {
 	    /* We know we're arriving only with invariant addresses here,
 	       try harder comparing them.  We can do some caching here
@@ -5957,8 +5969,6 @@ visit_phi (gimple *phi, bool *inserted, bool backedges_varying_p)
 	    sameval = NULL_TREE;
 	    break;
 	  }
-	else
-	  sameval_e = NULL;
       }
 
   /* If the value we want to use is flowing over the backedge and we
diff --git a/gcc/tree-ssa.cc b/gcc/tree-ssa.cc
index 5126020d40f..a5cad2d344e 100644
--- a/gcc/tree-ssa.cc
+++ b/gcc/tree-ssa.cc
@@ -1320,6 +1320,8 @@ ssa_undefined_value_p (tree t, bool partial)
 {
   gimple *def_stmt;
 
+  gcc_checking_assert (!virtual_operand_p (t));
+
   if (ssa_defined_default_def_p (t))
     return false;

                 reply	other threads:[~2023-02-17 11:31 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230217113158.2AC4A382E69E@sourceware.org \
    --to=rguenth@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).