public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org, Aldy Hernandez <aldyh@redhat.com>,
	Andrew MacLeod <amacleod@redhat.com>
Subject: [PATCH] phiopt: Drop SSA_NAME_RANGE_INFO in maybe equal case [PR108166]
Date: Thu, 22 Dec 2022 11:30:09 +0100	[thread overview]
Message-ID: <Y6QxsdUy2S2w//u/@tucnak> (raw)

Hi!

The following place in value_replacement is after proving that
x == cst1 ? cst2 : x
phi result is only used in a comparison with constant which doesn't
care if it compares cst1 or cst2 and replaces it with x.
The testcase is miscompiled because we have after the replacement
incorrect range info for the phi result, we would need to
effectively union the phi result range with cst1 (oarg in the code)
because previously that constant might be missing in the range, but
newly it can appear (we've just verified that the single use stmt
of the phi result doesn't care about that value in particular).

The following patch just resets the info, bootstrapped/regtested
on x86_64-linux and i686-linux, ok for trunk?

Aldy/Andrew, how would one instead union the SSA_NAME_RANGE_INFO
with some INTEGER_CST and store it back into SSA_NAME_RANGE_INFO
(including adjusting non-zero bits and the like)?

2022-12-22  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/108166
	* tree-ssa-phiopt.cc (value_replacement): For the maybe_equal_p
	case turned into equal_p reset SSA_NAME_RANGE_INFO of phi result.

	* g++.dg/torture/pr108166.C: New test.

--- gcc/tree-ssa-phiopt.cc.jj	2022-10-28 11:00:53.970243821 +0200
+++ gcc/tree-ssa-phiopt.cc	2022-12-21 14:27:58.118326548 +0100
@@ -1491,6 +1491,12 @@ value_replacement (basic_block cond_bb,
 		  default:
 		    break;
 		  }
+	      if (equal_p)
+		/* After the optimization PHI result can have value
+		   which it couldn't have previously.
+		   We could instead of resetting it union the range
+		   info with oarg.  */
+		reset_flow_sensitive_info (gimple_phi_result (phi));
 	      if (equal_p && MAY_HAVE_DEBUG_BIND_STMTS)
 		{
 		  imm_use_iterator imm_iter;
--- gcc/testsuite/g++.dg/torture/pr108166.C.jj	2022-12-21 14:31:02.638661322 +0100
+++ gcc/testsuite/g++.dg/torture/pr108166.C	2022-12-21 14:30:45.441909725 +0100
@@ -0,0 +1,26 @@
+// PR tree-optimization/108166
+// { dg-do run }
+
+bool a, b;
+int d, c;
+
+const int &
+foo (const int &f, const int &g)
+{
+  return !f ? f : g;
+}
+
+__attribute__((noipa)) void
+bar (int)
+{
+}
+
+int
+main ()
+{
+  c = foo (b, 0) > ((b ? d : b) ?: 8);
+  a = b ? d : b;
+  bar (a);
+  if (a != 0)
+    __builtin_abort ();
+}

	Jakub


             reply	other threads:[~2022-12-22 10:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-22 10:30 Jakub Jelinek [this message]
2022-12-22 11:13 ` Aldy Hernandez
2022-12-22 11:33 ` Richard Biener
2022-12-22 12:09   ` Aldy Hernandez
2022-12-22 12:54     ` [PATCH] phiopt: Adjust instead of reset phires range Jakub Jelinek
2022-12-22 19:46       ` Aldy Hernandez
2022-12-22 21:44         ` [PATCH] phiopt, v2: " Jakub Jelinek
2022-12-22 22:11           ` Aldy Hernandez

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=Y6QxsdUy2S2w//u/@tucnak \
    --to=jakub@redhat.com \
    --cc=aldyh@redhat.com \
    --cc=amacleod@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    /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).