public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [COMMITTED] Return TRUE only when a global value is updated.
@ 2023-10-03 14:31 Andrew MacLeod
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew MacLeod @ 2023-10-03 14:31 UTC (permalink / raw)
  To: gcc-patches

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

set_range_info should return TRUE only when it sets a new value. It was 
currently returning true whenever it set a value, whether it was 
different or not.

With this change,  VRP no longer overwrites global ranges DOM has set.  
2 testcases needed adjusting that were expecting VRP2 to set a range but 
turns out it was really being set in DOM2.   Instead they check for the 
range in the final listing...

Bootstrapped on  x86_64-pc-linux-gnu with no regressions. Pushed.

Andrew

[-- Attachment #2: 0001-Return-TRUE-only-when-a-global-value-is-updated.patch --]
[-- Type: text/x-patch, Size: 3929 bytes --]

From dae5de2a2353b928cc7099a78d88a40473abefd2 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Wed, 27 Sep 2023 12:34:16 -0400
Subject: [PATCH 1/5] Return TRUE only when a global value is updated.

set_range_info should return TRUE only when it sets a new value.  VRP no
longer overwrites global ranges DOM has set.  Check for ranges in the
final listing.

	gcc/
	* tree-ssanames.cc (set_range_info): Return true only if the
	current value changes.

	gcc/testsuite/
	* gcc.dg/pr93917.c: Check for ranges in final optimized listing.
	* gcc.dg/tree-ssa/vrp-unreachable.c: Ditto.
---
 gcc/testsuite/gcc.dg/pr93917.c                |  4 ++--
 .../gcc.dg/tree-ssa/vrp-unreachable.c         |  4 ++--
 gcc/tree-ssanames.cc                          | 24 +++++++++----------
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/pr93917.c b/gcc/testsuite/gcc.dg/pr93917.c
index f09e1c41ae8..f636b77f45d 100644
--- a/gcc/testsuite/gcc.dg/pr93917.c
+++ b/gcc/testsuite/gcc.dg/pr93917.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-vrp1 -fdump-tree-vrp2" } */
+/* { dg-options "-O2 -fdump-tree-vrp1 -fdump-tree-vrp2 -fdump-tree-optimized-alias" } */
 
 void f3(int n);
 
@@ -19,5 +19,5 @@ void f2(int*n)
 
 /* { dg-final { scan-tree-dump-times "Global Export.*0, \\+INF" 1 "vrp1" } } */
 /* { dg-final { scan-tree-dump-times "__builtin_unreachable" 1 "vrp1" } } */
-/* { dg-final { scan-tree-dump-times "Global Export.*0, \\+INF" 1 "vrp2" } } */
 /* { dg-final { scan-tree-dump-times "__builtin_unreachable" 0 "vrp2" } } */
+/* { dg-final { scan-tree-dump-times "0, \\+INF" 2 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/vrp-unreachable.c b/gcc/testsuite/gcc.dg/tree-ssa/vrp-unreachable.c
index 5835dfc8dbc..4aad7f1be5d 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/vrp-unreachable.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/vrp-unreachable.c
@@ -1,5 +1,5 @@
 /* { dg-do compile } */
-/* { dg-options "-O2 -fdump-tree-vrp1-alias -fdump-tree-vrp2-alias" } */
+/* { dg-options "-O2 -fdump-tree-vrp1 -fdump-tree-vrp2 -fdump-tree-optimized-alias" } */
 
 void dead (unsigned n);
 void alive (unsigned n);
@@ -39,4 +39,4 @@ void func (unsigned n, unsigned m)
 /* { dg-final { scan-tree-dump-not "dead" "vrp1" } } */
 /* { dg-final { scan-tree-dump-times "builtin_unreachable" 1 "vrp1" } } */
 /* { dg-final { scan-tree-dump-not "builtin_unreachable" "vrp2" } } */
-/* { dg-final { scan-tree-dump-times "fff8 VALUE 0x0" 4 "vrp2" } } */
+/* { dg-final { scan-tree-dump-times "fff8 VALUE 0x0" 2 "optimized" } } */
diff --git a/gcc/tree-ssanames.cc b/gcc/tree-ssanames.cc
index 23387b90fe3..1eae411ac1c 100644
--- a/gcc/tree-ssanames.cc
+++ b/gcc/tree-ssanames.cc
@@ -418,10 +418,17 @@ set_range_info (tree name, const vrange &r)
   if (r.undefined_p () || r.varying_p ())
     return false;
 
+  // Pick up the current range, or VARYING if none.
   tree type = TREE_TYPE (name);
+  Value_Range tmp (type);
+  if (range_info_p (name))
+    range_info_get_range (name, tmp);
+  else
+    tmp.set_varying (type);
+
   if (POINTER_TYPE_P (type))
     {
-      if (r.nonzero_p ())
+      if (r.nonzero_p () && !tmp.nonzero_p ())
 	{
 	  set_ptr_nonnull (name);
 	  return true;
@@ -429,18 +436,11 @@ set_range_info (tree name, const vrange &r)
       return false;
     }
 
-  /* If a global range already exists, incorporate it.  */
-  if (range_info_p (name))
-    {
-      Value_Range tmp (type);
-      range_info_get_range (name, tmp);
-      tmp.intersect (r);
-      if (tmp.undefined_p ())
-	return false;
+  // If the result doesn't change, or is undefined, return false.
+  if (!tmp.intersect (r) || tmp.undefined_p ())
+    return false;
 
-      return range_info_set_range (name, tmp);
-    }
-  return range_info_set_range (name, r);
+  return range_info_set_range (name, tmp);
 }
 
 /* Set nonnull attribute to pointer NAME.  */
-- 
2.41.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [COMMITTED] Return TRUE only when a global value is updated.
  2023-10-03 16:53   ` David Edelsohn
@ 2023-10-03 16:57     ` Andrew MacLeod
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew MacLeod @ 2023-10-03 16:57 UTC (permalink / raw)
  To: David Edelsohn; +Cc: GCC Patches

perfect.  I'll check it in when my testrun is done.

Thanks  .. .  and sorry :-)

Andrew

On 10/3/23 12:53, David Edelsohn wrote:
> AIX bootstrap is happier with the patch.
>
> Thanks, David
>
> On Tue, Oct 3, 2023 at 12:30 PM Andrew MacLeod <amacleod@redhat.com> 
> wrote:
>
>     Give this a try..  I'm testing it here, but x86 doesn't seem to
>     show it
>     anyway for some reason :-P
>
>     I think i needed to handle pointers special since SSA_NAMES handle
>     pointer ranges different.
>
>     Andrew
>
>     On 10/3/23 11:47, David Edelsohn wrote:
>     > This patch caused a bootstrap failure on AIX.
>     >
>     > during GIMPLE pass: evrp
>     >
>     > /nasfarm/edelsohn/src/src/libgcc/libgcc2.c: In function
>     '__gcc_bcmp':
>     >
>     > /nasfarm/edelsohn/src/src/libgcc/libgcc2.c:2910:1: internal
>     compiler
>     > error: in get_irange, at value-range-storage.cc:343
>     >
>     > 2910 | }
>     >
>     > | ^
>     >
>     >
>     > 0x11b7f4b7 irange_storage::get_irange(irange&, tree_node*) const
>     >
>     > /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:343
>     >
>     > 0x11b7e7af vrange_storage::get_vrange(vrange&, tree_node*) const
>     >
>     > /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:178
>     >
>     > 0x139f3d77 range_info_get_range(tree_node const*, vrange&)
>     >
>     > /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:118
>     >
>     > 0x1134b463 set_range_info(tree_node*, vrange const&)
>     >
>     > /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:425
>     >
>     > 0x116a7333 gimple_ranger::register_inferred_ranges(gimple*)
>     >
>     > /nasfarm/edelsohn/src/src/gcc/gimple-range.cc:487
>     >
>     > 0x125cef27 rvrp_folder::fold_stmt(gimple_stmt_iterator*)
>     >
>     > /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1033
>     >
>     > 0x123dd063
>     >
>     substitute_and_fold_dom_walker::before_dom_children(basic_block_def*)
>     >
>     > /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:876
>     >
>     > 0x1176cc43 dom_walker::walk(basic_block_def*)
>     >
>     > /nasfarm/edelsohn/src/src/gcc/domwalk.cc:311
>     >
>     > 0x123dd733
>     > substitute_and_fold_engine::substitute_and_fold(basic_block_def*)
>     >
>     > /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:999
>     >
>     > 0x123d0f5f execute_ranger_vrp(function*, bool, bool)
>     >
>     > /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1062
>     >
>     > 0x123d14ef execute
>     >
>     > /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1142
>     >
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [COMMITTED] Return TRUE only when a global value is updated.
  2023-10-03 16:30 ` Andrew MacLeod
@ 2023-10-03 16:53   ` David Edelsohn
  2023-10-03 16:57     ` Andrew MacLeod
  0 siblings, 1 reply; 6+ messages in thread
From: David Edelsohn @ 2023-10-03 16:53 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: GCC Patches

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

AIX bootstrap is happier with the patch.

Thanks, David

On Tue, Oct 3, 2023 at 12:30 PM Andrew MacLeod <amacleod@redhat.com> wrote:

> Give this a try..  I'm testing it here, but x86 doesn't seem to show it
> anyway for some reason :-P
>
> I think i needed to handle pointers special since SSA_NAMES handle
> pointer ranges different.
>
> Andrew
>
> On 10/3/23 11:47, David Edelsohn wrote:
> > This patch caused a bootstrap failure on AIX.
> >
> > during GIMPLE pass: evrp
> >
> > /nasfarm/edelsohn/src/src/libgcc/libgcc2.c: In function '__gcc_bcmp':
> >
> > /nasfarm/edelsohn/src/src/libgcc/libgcc2.c:2910:1: internal compiler
> > error: in get_irange, at value-range-storage.cc:343
> >
> > 2910 | }
> >
> > | ^
> >
> >
> > 0x11b7f4b7 irange_storage::get_irange(irange&, tree_node*) const
> >
> > /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:343
> >
> > 0x11b7e7af vrange_storage::get_vrange(vrange&, tree_node*) const
> >
> > /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:178
> >
> > 0x139f3d77 range_info_get_range(tree_node const*, vrange&)
> >
> > /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:118
> >
> > 0x1134b463 set_range_info(tree_node*, vrange const&)
> >
> > /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:425
> >
> > 0x116a7333 gimple_ranger::register_inferred_ranges(gimple*)
> >
> > /nasfarm/edelsohn/src/src/gcc/gimple-range.cc:487
> >
> > 0x125cef27 rvrp_folder::fold_stmt(gimple_stmt_iterator*)
> >
> > /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1033
> >
> > 0x123dd063
> > substitute_and_fold_dom_walker::before_dom_children(basic_block_def*)
> >
> > /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:876
> >
> > 0x1176cc43 dom_walker::walk(basic_block_def*)
> >
> > /nasfarm/edelsohn/src/src/gcc/domwalk.cc:311
> >
> > 0x123dd733
> > substitute_and_fold_engine::substitute_and_fold(basic_block_def*)
> >
> > /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:999
> >
> > 0x123d0f5f execute_ranger_vrp(function*, bool, bool)
> >
> > /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1062
> >
> > 0x123d14ef execute
> >
> > /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1142
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [COMMITTED] Return TRUE only when a global value is updated.
  2023-10-03 15:47 David Edelsohn
  2023-10-03 15:55 ` Andrew MacLeod
@ 2023-10-03 16:30 ` Andrew MacLeod
  2023-10-03 16:53   ` David Edelsohn
  1 sibling, 1 reply; 6+ messages in thread
From: Andrew MacLeod @ 2023-10-03 16:30 UTC (permalink / raw)
  To: David Edelsohn; +Cc: GCC Patches

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

Give this a try..  I'm testing it here, but x86 doesn't seem to show it 
anyway for some reason :-P

I think i needed to handle pointers special since SSA_NAMES handle 
pointer ranges different.

Andrew

On 10/3/23 11:47, David Edelsohn wrote:
> This patch caused a bootstrap failure on AIX.
>
> during GIMPLE pass: evrp
>
> /nasfarm/edelsohn/src/src/libgcc/libgcc2.c: In function '__gcc_bcmp':
>
> /nasfarm/edelsohn/src/src/libgcc/libgcc2.c:2910:1: internal compiler 
> error: in get_irange, at value-range-storage.cc:343
>
> 2910 | }
>
> | ^
>
>
> 0x11b7f4b7 irange_storage::get_irange(irange&, tree_node*) const
>
> /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:343
>
> 0x11b7e7af vrange_storage::get_vrange(vrange&, tree_node*) const
>
> /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:178
>
> 0x139f3d77 range_info_get_range(tree_node const*, vrange&)
>
> /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:118
>
> 0x1134b463 set_range_info(tree_node*, vrange const&)
>
> /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:425
>
> 0x116a7333 gimple_ranger::register_inferred_ranges(gimple*)
>
> /nasfarm/edelsohn/src/src/gcc/gimple-range.cc:487
>
> 0x125cef27 rvrp_folder::fold_stmt(gimple_stmt_iterator*)
>
> /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1033
>
> 0x123dd063 
> substitute_and_fold_dom_walker::before_dom_children(basic_block_def*)
>
> /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:876
>
> 0x1176cc43 dom_walker::walk(basic_block_def*)
>
> /nasfarm/edelsohn/src/src/gcc/domwalk.cc:311
>
> 0x123dd733 
> substitute_and_fold_engine::substitute_and_fold(basic_block_def*)
>
> /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:999
>
> 0x123d0f5f execute_ranger_vrp(function*, bool, bool)
>
> /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1062
>
> 0x123d14ef execute
>
> /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1142
>

[-- Attachment #2: ptr.diff --]
[-- Type: text/x-patch, Size: 1091 bytes --]

diff --git a/gcc/tree-ssanames.cc b/gcc/tree-ssanames.cc
index 1eae411ac1c..1401f67c781 100644
--- a/gcc/tree-ssanames.cc
+++ b/gcc/tree-ssanames.cc
@@ -420,15 +420,11 @@ set_range_info (tree name, const vrange &r)
 
   // Pick up the current range, or VARYING if none.
   tree type = TREE_TYPE (name);
-  Value_Range tmp (type);
-  if (range_info_p (name))
-    range_info_get_range (name, tmp);
-  else
-    tmp.set_varying (type);
-
   if (POINTER_TYPE_P (type))
     {
-      if (r.nonzero_p () && !tmp.nonzero_p ())
+      struct ptr_info_def *pi = get_ptr_info (name);
+      // If R is nonnull and pi is not, set nonnull.
+      if (r.nonzero_p () && (!pi || !pi->pt.null))
 	{
 	  set_ptr_nonnull (name);
 	  return true;
@@ -436,6 +432,11 @@ set_range_info (tree name, const vrange &r)
       return false;
     }
 
+  Value_Range tmp (type);
+  if (range_info_p (name))
+    range_info_get_range (name, tmp);
+  else
+    tmp.set_varying (type);
   // If the result doesn't change, or is undefined, return false.
   if (!tmp.intersect (r) || tmp.undefined_p ())
     return false;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [COMMITTED] Return TRUE only when a global value is updated.
  2023-10-03 15:47 David Edelsohn
@ 2023-10-03 15:55 ` Andrew MacLeod
  2023-10-03 16:30 ` Andrew MacLeod
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew MacLeod @ 2023-10-03 15:55 UTC (permalink / raw)
  To: David Edelsohn; +Cc: GCC Patches

huh.  thanks,  I'll have a look.


Andrew

On 10/3/23 11:47, David Edelsohn wrote:
> This patch caused a bootstrap failure on AIX.
>
> during GIMPLE pass: evrp
>
> /nasfarm/edelsohn/src/src/libgcc/libgcc2.c: In function '__gcc_bcmp':
>
> /nasfarm/edelsohn/src/src/libgcc/libgcc2.c:2910:1: internal compiler 
> error: in get_irange, at value-range-storage.cc:343
>
> 2910 | }
>
> | ^
>
>
> 0x11b7f4b7 irange_storage::get_irange(irange&, tree_node*) const
>
> /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:343
>
> 0x11b7e7af vrange_storage::get_vrange(vrange&, tree_node*) const
>
> /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:178
>
> 0x139f3d77 range_info_get_range(tree_node const*, vrange&)
>
> /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:118
>
> 0x1134b463 set_range_info(tree_node*, vrange const&)
>
> /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:425
>
> 0x116a7333 gimple_ranger::register_inferred_ranges(gimple*)
>
> /nasfarm/edelsohn/src/src/gcc/gimple-range.cc:487
>
> 0x125cef27 rvrp_folder::fold_stmt(gimple_stmt_iterator*)
>
> /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1033
>
> 0x123dd063 
> substitute_and_fold_dom_walker::before_dom_children(basic_block_def*)
>
> /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:876
>
> 0x1176cc43 dom_walker::walk(basic_block_def*)
>
> /nasfarm/edelsohn/src/src/gcc/domwalk.cc:311
>
> 0x123dd733 
> substitute_and_fold_engine::substitute_and_fold(basic_block_def*)
>
> /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:999
>
> 0x123d0f5f execute_ranger_vrp(function*, bool, bool)
>
> /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1062
>
> 0x123d14ef execute
>
> /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1142
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [COMMITTED] Return TRUE only when a global value is updated.
@ 2023-10-03 15:47 David Edelsohn
  2023-10-03 15:55 ` Andrew MacLeod
  2023-10-03 16:30 ` Andrew MacLeod
  0 siblings, 2 replies; 6+ messages in thread
From: David Edelsohn @ 2023-10-03 15:47 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: GCC Patches

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

This patch caused a bootstrap failure on AIX.

during GIMPLE pass: evrp

/nasfarm/edelsohn/src/src/libgcc/libgcc2.c: In function '__gcc_bcmp':

/nasfarm/edelsohn/src/src/libgcc/libgcc2.c:2910:1: internal compiler error:
in get_irange, at value-range-storage.cc:343

 2910 | }

      | ^


0x11b7f4b7 irange_storage::get_irange(irange&, tree_node*) const

        /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:343

0x11b7e7af vrange_storage::get_vrange(vrange&, tree_node*) const

        /nasfarm/edelsohn/src/src/gcc/value-range-storage.cc:178

0x139f3d77 range_info_get_range(tree_node const*, vrange&)

        /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:118

0x1134b463 set_range_info(tree_node*, vrange const&)

        /nasfarm/edelsohn/src/src/gcc/tree-ssanames.cc:425

0x116a7333 gimple_ranger::register_inferred_ranges(gimple*)

        /nasfarm/edelsohn/src/src/gcc/gimple-range.cc:487

0x125cef27 rvrp_folder::fold_stmt(gimple_stmt_iterator*)

        /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1033

0x123dd063
substitute_and_fold_dom_walker::before_dom_children(basic_block_def*)

        /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:876

0x1176cc43 dom_walker::walk(basic_block_def*)

        /nasfarm/edelsohn/src/src/gcc/domwalk.cc:311

0x123dd733 substitute_and_fold_engine::substitute_and_fold(basic_block_def*)

        /nasfarm/edelsohn/src/src/gcc/tree-ssa-propagate.cc:999

0x123d0f5f execute_ranger_vrp(function*, bool, bool)

        /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1062

0x123d14ef execute

        /nasfarm/edelsohn/src/src/gcc/tree-vrp.cc:1142

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-10-03 16:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-03 14:31 [COMMITTED] Return TRUE only when a global value is updated Andrew MacLeod
2023-10-03 15:47 David Edelsohn
2023-10-03 15:55 ` Andrew MacLeod
2023-10-03 16:30 ` Andrew MacLeod
2023-10-03 16:53   ` David Edelsohn
2023-10-03 16:57     ` Andrew MacLeod

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).