public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew MacLeod <amacleod@redhat.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
Cc: "hernandez, aldy" <aldyh@redhat.com>
Subject: [COMMITTED] PR tree-optimization/110582 - fur_list should not use the range vector for non-ssa, operands.
Date: Mon, 31 Jul 2023 12:06:40 -0400	[thread overview]
Message-ID: <73a27325-37f9-255e-4902-4adf41f5f4a6@redhat.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 2445 bytes --]

The fold_using_range operand fetching mechanism has a variety of modes.  
The "normal" mechanism simply invokes the current or supplied 
range_query to satisfy fetching current range info for any ssa-names 
used during the evalaution of the statement,

I also added support for fur_list which allows a list of ranges to be 
supplied which is used to satisfy ssa-names as they appear in the stmt.  
Once the list is exhausted, then it reverts to using the range query.

This allows us to fold a stmt using whatever values we want. ie,

a_2 = b_3 + c_4


i can call fold_stmt (r, stmt, [1,2],  [4,5])

and a_2 would be calculated using [1,2] for the first ssa_name, and 
[4,5] for the second encountered name.  This allows us to manually fold 
stmts when we desire.

There was a bug in the implementation of fur_list where it was using the 
supplied values for *any* encountered operand, not just ssa_names.

The PHI analyzer is the first consumer of the fur_list API, and was 
tripping over this.


     <bb 3> [local count: 1052266993]:
   # a_lsm.12_29 = PHI <iftmp.1_11(4)>
   iftmp.1_15 = 3 / a_lsm.12_29;

   <bb 4> [local count: 1063004408]:
   # iftmp.1_11 = PHI <iftmp.1_15(3), 2(2)>
   # ivtmp_2 = PHI <ivtmp_14(3), 253(2)>
   ivtmp_36 = ivtmp_2 - 1;
   if (ivtmp_36 != 0)
     goto <bb 3>; [98.99%]
   else
     goto <bb 5>; [1.01%]

It detemined that the initial value of iftmp.1_11 was [2, 2] (from the 
edge 2->4), and that the only modifying statement is
iftmp.1_15 = 3 / a_lsm.12_29;

One of the things it tries to do is determine is if a few iterations 
feeding the initial value and combining it with the result of the 
statement converge, thus providing a complete initial range.  Its uses 
fold_range supplying the value for the ssa-operand directly..  but 
tripped over the bug.

So for the first iteration, instead of calculating   _15 = 3 / [2,2]  
and coming up with [1,1],   it was instead calculating [2,2]/VARYING, 
and coming up with [-2, 2].  Next pass of the iteration checker then 
erroneously calculated [-2,2]/VARYING and the result was [-2,2] and 
convergence was achieved, and the initial value of the PHI set to[-2, 2] 
... incorrectly.  and of course bad things happened.

This patch fixes fur_list::get_operand to check for an ssa-name before 
it pulling a value from the supplied list.  With this, no partlculary 
good starting value for the PHI node can be determined.

Andrew


[-- Attachment #2: 582.patch --]
[-- Type: text/x-patch, Size: 1812 bytes --]

From 914fa35a7f7db76211ca259606578193773a254e Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Mon, 31 Jul 2023 10:08:51 -0400
Subject: [PATCH] fur_list should not use the range vector for non-ssa
 operands.

	gcc/
	PR tree-optimization/110582
	* gimple-range-fold.cc (fur_list::get_operand): Do not use the
	range vector for non-ssa names.

	gcc/testsuite/
	* gcc.dg/pr110582.c: New.
---
 gcc/gimple-range-fold.cc        |  3 ++-
 gcc/testsuite/gcc.dg/pr110582.c | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr110582.c

diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
index d07246008f0..ab2d996c4eb 100644
--- a/gcc/gimple-range-fold.cc
+++ b/gcc/gimple-range-fold.cc
@@ -262,7 +262,8 @@ fur_list::fur_list (unsigned num, vrange **list, range_query *q)
 bool
 fur_list::get_operand (vrange &r, tree expr)
 {
-  if (m_index >= m_limit)
+  // Do not use the vector for non-ssa-names, or if it has been emptied.
+  if (TREE_CODE (expr) != SSA_NAME || m_index >= m_limit)
     return m_query->range_of_expr (r, expr);
   r = *m_list[m_index++];
   gcc_checking_assert (range_compatible_p (TREE_TYPE (expr), r.type ()));
diff --git a/gcc/testsuite/gcc.dg/pr110582.c b/gcc/testsuite/gcc.dg/pr110582.c
new file mode 100644
index 00000000000..ae0650d3ae7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr110582.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-vrp2" } */
+
+int a, b;
+int main() {
+  char c = a = 0;
+  for (; c != -3; c++) {
+    int d = 2;
+    d ^= 2 && a;
+    b = a == 0 ? d : d / a;
+    a = b;
+  }
+  for (; (1 + 95 << 24) + b + 1 + 686658714L + b - 2297271457;)
+    ;
+}
+
+/* { dg-final { scan-tree-dump-not "Folding predicate" "vrp2" } } */
+
-- 
2.40.1


                 reply	other threads:[~2023-07-31 16:06 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=73a27325-37f9-255e-4902-4adf41f5f4a6@redhat.com \
    --to=amacleod@redhat.com \
    --cc=aldyh@redhat.com \
    --cc=gcc-patches@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).