diff mbox

Clear SSA_NAME_ANTI_RANGE_P when appropriate (PR tree-optimization/67821)

Message ID 20151005142544.GL6184@redhat.com
State New
Headers show

Commit Message

Marek Polacek Oct. 5, 2015, 2:25 p.m. UTC
On Mon, Oct 05, 2015 at 04:08:05PM +0200, Richard Biener wrote:
> On Mon, 5 Oct 2015, Marek Polacek wrote:
> 
> > Here, we were crashing on an assert in duplicate_ssa_name_range_info:
> > 
> > 506   gcc_assert (!SSA_NAME_ANTI_RANGE_P (name));
> > 
> > The problem is that reset_flow_sensitive_info wasn't clearing the
> > SSA_NAME_ANTI_RANGE_P flag; I don't think NULL SSA_NAME_RANGE_INFO
> > can ever describe an anti-range...
> 
> Yeah, also not a range.  Thus I think the assert is bogus (aka
> superfluous) if !SSA_NAME_RANGE_INFO (name) holds.

Hm, true, and duplicate_ssa_name_range_info unconditionally sets
SSA_NAME_ANTI_RANGE_P... 

> Otherwise other setters of SSA_NAME_RANGE_INFO would need to make
> sure SSA_NAME_ANTI_RANGE_P is cleared as well.
 
They mostly do, if I'm looking right, e.g.
tree-ssa-phiopt.c:1016
tree-ssa-loop-im.c:1224
tree-ssa-loop-im.c:1294
and I've also seen
<https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00127.html>.

> So - can you instead remove the assert?

Sure.  So is the following ok once it passes the usual testing?

2015-10-05  Marek Polacek  <polacek@redhat.com>

	PR tree-optimization/67821
	* tree-ssanames.c (duplicate_ssa_name_range_info): Remove an assert.

	* gcc.dg/torture/pr67821-2.c: New test.
	* gcc.dg/torture/pr67821.c: New test.


	Marek

Comments

Richard Biener Oct. 5, 2015, 2:26 p.m. UTC | #1
On Mon, 5 Oct 2015, Marek Polacek wrote:

> On Mon, Oct 05, 2015 at 04:08:05PM +0200, Richard Biener wrote:
> > On Mon, 5 Oct 2015, Marek Polacek wrote:
> > 
> > > Here, we were crashing on an assert in duplicate_ssa_name_range_info:
> > > 
> > > 506   gcc_assert (!SSA_NAME_ANTI_RANGE_P (name));
> > > 
> > > The problem is that reset_flow_sensitive_info wasn't clearing the
> > > SSA_NAME_ANTI_RANGE_P flag; I don't think NULL SSA_NAME_RANGE_INFO
> > > can ever describe an anti-range...
> > 
> > Yeah, also not a range.  Thus I think the assert is bogus (aka
> > superfluous) if !SSA_NAME_RANGE_INFO (name) holds.
> 
> Hm, true, and duplicate_ssa_name_range_info unconditionally sets
> SSA_NAME_ANTI_RANGE_P... 
> 
> > Otherwise other setters of SSA_NAME_RANGE_INFO would need to make
> > sure SSA_NAME_ANTI_RANGE_P is cleared as well.
>  
> They mostly do, if I'm looking right, e.g.
> tree-ssa-phiopt.c:1016
> tree-ssa-loop-im.c:1224
> tree-ssa-loop-im.c:1294
> and I've also seen
> <https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00127.html>.

Those would be redundant then.

> > So - can you instead remove the assert?
> 
> Sure.  So is the following ok once it passes the usual testing?

Yes.

Thanks,
Richard.

> 2015-10-05  Marek Polacek  <polacek@redhat.com>
> 
> 	PR tree-optimization/67821
> 	* tree-ssanames.c (duplicate_ssa_name_range_info): Remove an assert.
> 
> 	* gcc.dg/torture/pr67821-2.c: New test.
> 	* gcc.dg/torture/pr67821.c: New test.
> 
> diff --git gcc/testsuite/gcc.dg/torture/pr67821-2.c gcc/testsuite/gcc.dg/torture/pr67821-2.c
> index e69de29..38cfc84 100644
> --- gcc/testsuite/gcc.dg/torture/pr67821-2.c
> +++ gcc/testsuite/gcc.dg/torture/pr67821-2.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +
> +int a, b, c, d, e, g;
> +short f;
> +
> +void
> +fn1 ()
> +{
> +  int i;
> +  f = a - b;
> +  e = (c && (i = d = (unsigned) f - 1)) || i;
> +  g = (unsigned) f - 1;
> +  c && (d = 0);
> +}
> diff --git gcc/testsuite/gcc.dg/torture/pr67821.c gcc/testsuite/gcc.dg/torture/pr67821.c
> index e69de29..1c9e8b9 100644
> --- gcc/testsuite/gcc.dg/torture/pr67821.c
> +++ gcc/testsuite/gcc.dg/torture/pr67821.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +
> +int isdigit (int);
> +
> +int
> +foo (const char *s)
> +{
> +  int success = 1;
> +  const char *p = s + 2;
> +  if (!isdigit (*p))
> +    success = 0;
> +  while (isdigit (*p))
> +    ++p;
> +  return success;
> +}
> diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c
> index 64e2379..91f4ed8 100644
> --- gcc/tree-ssanames.c
> +++ gcc/tree-ssanames.c
> @@ -503,7 +503,6 @@ duplicate_ssa_name_range_info (tree name, enum value_range_type range_type,
>  
>    gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name)));
>    gcc_assert (!SSA_NAME_RANGE_INFO (name));
> -  gcc_assert (!SSA_NAME_ANTI_RANGE_P (name));
>  
>    if (!range_info)
>      return;
> 
> 	Marek
> 
>
diff mbox

Patch

diff --git gcc/testsuite/gcc.dg/torture/pr67821-2.c gcc/testsuite/gcc.dg/torture/pr67821-2.c
index e69de29..38cfc84 100644
--- gcc/testsuite/gcc.dg/torture/pr67821-2.c
+++ gcc/testsuite/gcc.dg/torture/pr67821-2.c
@@ -0,0 +1,14 @@ 
+/* { dg-do compile } */
+
+int a, b, c, d, e, g;
+short f;
+
+void
+fn1 ()
+{
+  int i;
+  f = a - b;
+  e = (c && (i = d = (unsigned) f - 1)) || i;
+  g = (unsigned) f - 1;
+  c && (d = 0);
+}
diff --git gcc/testsuite/gcc.dg/torture/pr67821.c gcc/testsuite/gcc.dg/torture/pr67821.c
index e69de29..1c9e8b9 100644
--- gcc/testsuite/gcc.dg/torture/pr67821.c
+++ gcc/testsuite/gcc.dg/torture/pr67821.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+
+int isdigit (int);
+
+int
+foo (const char *s)
+{
+  int success = 1;
+  const char *p = s + 2;
+  if (!isdigit (*p))
+    success = 0;
+  while (isdigit (*p))
+    ++p;
+  return success;
+}
diff --git gcc/tree-ssanames.c gcc/tree-ssanames.c
index 64e2379..91f4ed8 100644
--- gcc/tree-ssanames.c
+++ gcc/tree-ssanames.c
@@ -503,7 +503,6 @@  duplicate_ssa_name_range_info (tree name, enum value_range_type range_type,
 
   gcc_assert (!POINTER_TYPE_P (TREE_TYPE (name)));
   gcc_assert (!SSA_NAME_RANGE_INFO (name));
-  gcc_assert (!SSA_NAME_ANTI_RANGE_P (name));
 
   if (!range_info)
     return;