public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] gimple-range-cache: Fix ICEs when dumping details [PR111967]
@ 2023-11-11  8:35 Jakub Jelinek
  2023-11-13  7:02 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-11-11  8:35 UTC (permalink / raw)
  To: Andrew MacLeod; +Cc: gcc-patches

Hi!

The following testcase ICEs when dumping details.
When m_ssa_ranges vector is created, it is safe_grow_cleared (num_ssa_names),
but when when some new SSA_NAME is added, we strangely grow it to
num_ssa_names + 1 instead and later on the 3 argument dump method
iterates from 1 to m_ssa_ranges.length () - 1 and uses ssa_name (x)
on each; but because set_bb_range grew it one too much, ssa_name
(m_ssa_ranges.length () - 1) might be after the end of the ssanames
vector and ICE.

The fix grows the vector consistently only to num_ssa_names,
doesn't waste time checking m_ssa_ranges[0] because there is no
ssa_names (0), it is always NULL, before using ssa_name (x) checks
if we'll need it at all (we check later if m_ssa_ranges[x] is non-NULL,
so we might check it earlier as well) and also in the last loop
iterates until m_ssa_ranges.length () rather than num_ssa_names, I don't
see a reason for the inconsistency and in theory some SSA_NAME could be
added without set_bb_range called for it and the vector could be shorter
than the ssanames vector.

To actually fix the ICE, either the first hunk or the last 2 hunks
would be enough, but I think it doesn't hurt to change all the spots.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-11-11  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/111967
	* gimple-range-cache.cc (block_range_cache::set_bb_range): Grow
	m_ssa_ranges to num_ssa_names rather than num_ssa_names + 1.
	(block_range_cache::dump): Iterate from 1 rather than 0.  Don't use
	ssa_name (x) unless m_ssa_ranges[x] is non-NULL.  Iterate to
	m_ssa_ranges.length () rather than num_ssa_names.

	* gcc.dg/tree-ssa/pr111967.c: New test.

--- gcc/gimple-range-cache.cc.jj	2023-10-10 11:56:05.819220320 +0200
+++ gcc/gimple-range-cache.cc	2023-11-10 17:06:52.482867324 +0100
@@ -390,7 +390,7 @@ block_range_cache::set_bb_range (tree na
 {
   unsigned v = SSA_NAME_VERSION (name);
   if (v >= m_ssa_ranges.length ())
-    m_ssa_ranges.safe_grow_cleared (num_ssa_names + 1);
+    m_ssa_ranges.safe_grow_cleared (num_ssa_names);
 
   if (!m_ssa_ranges[v])
     {
@@ -465,7 +465,7 @@ void
 block_range_cache::dump (FILE *f)
 {
   unsigned x;
-  for (x = 0; x < m_ssa_ranges.length (); ++x)
+  for (x = 1; x < m_ssa_ranges.length (); ++x)
     {
       if (m_ssa_ranges[x])
 	{
@@ -487,11 +487,14 @@ block_range_cache::dump (FILE *f, basic_
   bool summarize_varying = false;
   for (x = 1; x < m_ssa_ranges.length (); ++x)
     {
+      if (!m_ssa_ranges[x])
+	continue;
+
       if (!gimple_range_ssa_p (ssa_name (x)))
 	continue;
 
       Value_Range r (TREE_TYPE (ssa_name (x)));
-      if (m_ssa_ranges[x] && m_ssa_ranges[x]->get_bb_range (r, bb))
+      if (m_ssa_ranges[x]->get_bb_range (r, bb))
 	{
 	  if (!print_varying && r.varying_p ())
 	    {
@@ -508,13 +511,16 @@ block_range_cache::dump (FILE *f, basic_
   if (summarize_varying)
     {
       fprintf (f, "VARYING_P on entry : ");
-      for (x = 1; x < num_ssa_names; ++x)
+      for (x = 1; x < m_ssa_ranges.length (); ++x)
 	{
+	  if (!m_ssa_ranges[x])
+	    continue;
+
 	  if (!gimple_range_ssa_p (ssa_name (x)))
 	    continue;
 
 	  Value_Range r (TREE_TYPE (ssa_name (x)));
-	  if (m_ssa_ranges[x] && m_ssa_ranges[x]->get_bb_range (r, bb))
+	  if (m_ssa_ranges[x]->get_bb_range (r, bb))
 	    {
 	      if (r.varying_p ())
 		{
--- gcc/testsuite/gcc.dg/tree-ssa/pr111967.c.jj	2023-11-10 16:45:54.006085324 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr111967.c	2023-11-10 17:03:17.257844360 +0100
@@ -0,0 +1,15 @@
+/* PR tree-optimization/111967 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-tree-forwprop -fdump-tree-evrp-all" } */
+
+void bar (char *);
+int a;
+char *b;
+
+void
+foo (void)
+{
+  long c = a & 3;
+  if (c)
+    bar (b + c);
+}

	Jakub


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

* Re: [PATCH] gimple-range-cache: Fix ICEs when dumping details [PR111967]
  2023-11-11  8:35 [PATCH] gimple-range-cache: Fix ICEs when dumping details [PR111967] Jakub Jelinek
@ 2023-11-13  7:02 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-11-13  7:02 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Andrew MacLeod, gcc-patches

On Sat, Nov 11, 2023 at 9:36 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The following testcase ICEs when dumping details.
> When m_ssa_ranges vector is created, it is safe_grow_cleared (num_ssa_names),
> but when when some new SSA_NAME is added, we strangely grow it to
> num_ssa_names + 1 instead and later on the 3 argument dump method
> iterates from 1 to m_ssa_ranges.length () - 1 and uses ssa_name (x)
> on each; but because set_bb_range grew it one too much, ssa_name
> (m_ssa_ranges.length () - 1) might be after the end of the ssanames
> vector and ICE.
>
> The fix grows the vector consistently only to num_ssa_names,
> doesn't waste time checking m_ssa_ranges[0] because there is no
> ssa_names (0), it is always NULL, before using ssa_name (x) checks
> if we'll need it at all (we check later if m_ssa_ranges[x] is non-NULL,
> so we might check it earlier as well) and also in the last loop
> iterates until m_ssa_ranges.length () rather than num_ssa_names, I don't
> see a reason for the inconsistency and in theory some SSA_NAME could be
> added without set_bb_range called for it and the vector could be shorter
> than the ssanames vector.
>
> To actually fix the ICE, either the first hunk or the last 2 hunks
> would be enough, but I think it doesn't hurt to change all the spots.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> 2023-11-11  Jakub Jelinek  <jakub@redhat.com>
>
>         PR tree-optimization/111967
>         * gimple-range-cache.cc (block_range_cache::set_bb_range): Grow
>         m_ssa_ranges to num_ssa_names rather than num_ssa_names + 1.
>         (block_range_cache::dump): Iterate from 1 rather than 0.  Don't use
>         ssa_name (x) unless m_ssa_ranges[x] is non-NULL.  Iterate to
>         m_ssa_ranges.length () rather than num_ssa_names.
>
>         * gcc.dg/tree-ssa/pr111967.c: New test.
>
> --- gcc/gimple-range-cache.cc.jj        2023-10-10 11:56:05.819220320 +0200
> +++ gcc/gimple-range-cache.cc   2023-11-10 17:06:52.482867324 +0100
> @@ -390,7 +390,7 @@ block_range_cache::set_bb_range (tree na
>  {
>    unsigned v = SSA_NAME_VERSION (name);
>    if (v >= m_ssa_ranges.length ())
> -    m_ssa_ranges.safe_grow_cleared (num_ssa_names + 1);
> +    m_ssa_ranges.safe_grow_cleared (num_ssa_names);
>
>    if (!m_ssa_ranges[v])
>      {
> @@ -465,7 +465,7 @@ void
>  block_range_cache::dump (FILE *f)
>  {
>    unsigned x;
> -  for (x = 0; x < m_ssa_ranges.length (); ++x)
> +  for (x = 1; x < m_ssa_ranges.length (); ++x)
>      {
>        if (m_ssa_ranges[x])
>         {
> @@ -487,11 +487,14 @@ block_range_cache::dump (FILE *f, basic_
>    bool summarize_varying = false;
>    for (x = 1; x < m_ssa_ranges.length (); ++x)
>      {
> +      if (!m_ssa_ranges[x])
> +       continue;
> +
>        if (!gimple_range_ssa_p (ssa_name (x)))
>         continue;
>
>        Value_Range r (TREE_TYPE (ssa_name (x)));
> -      if (m_ssa_ranges[x] && m_ssa_ranges[x]->get_bb_range (r, bb))
> +      if (m_ssa_ranges[x]->get_bb_range (r, bb))
>         {
>           if (!print_varying && r.varying_p ())
>             {
> @@ -508,13 +511,16 @@ block_range_cache::dump (FILE *f, basic_
>    if (summarize_varying)
>      {
>        fprintf (f, "VARYING_P on entry : ");
> -      for (x = 1; x < num_ssa_names; ++x)
> +      for (x = 1; x < m_ssa_ranges.length (); ++x)
>         {
> +         if (!m_ssa_ranges[x])
> +           continue;
> +
>           if (!gimple_range_ssa_p (ssa_name (x)))
>             continue;
>
>           Value_Range r (TREE_TYPE (ssa_name (x)));
> -         if (m_ssa_ranges[x] && m_ssa_ranges[x]->get_bb_range (r, bb))
> +         if (m_ssa_ranges[x]->get_bb_range (r, bb))
>             {
>               if (r.varying_p ())
>                 {
> --- gcc/testsuite/gcc.dg/tree-ssa/pr111967.c.jj 2023-11-10 16:45:54.006085324 +0100
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr111967.c    2023-11-10 17:03:17.257844360 +0100
> @@ -0,0 +1,15 @@
> +/* PR tree-optimization/111967 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-forwprop -fdump-tree-evrp-all" } */
> +
> +void bar (char *);
> +int a;
> +char *b;
> +
> +void
> +foo (void)
> +{
> +  long c = a & 3;
> +  if (c)
> +    bar (b + c);
> +}
>
>         Jakub
>

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

end of thread, other threads:[~2023-11-13  7:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-11  8:35 [PATCH] gimple-range-cache: Fix ICEs when dumping details [PR111967] Jakub Jelinek
2023-11-13  7:02 ` Richard Biener

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