public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C++ PATCH] Partial fix for a recent regression (PR c++/90947)
@ 2019-10-22 15:20 Jakub Jelinek
  2019-10-29 20:26 ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2019-10-22 15:20 UTC (permalink / raw)
  To: Jason Merrill, Martin Sebor; +Cc: gcc-patches

Hi!

The following patch is just a partial fix for a regression introduced
in the PR90947 changes, the testcase is fixed for just C++17/20.
type_initializer_zero_p has been added to the generic code, supposedly
because similar initializer_zerop is in generic code too, but that
means it has a hand-written copy of next_initializable_field which can't
do exactly what next_initializable_field does.

The following patch moves it into the C++ FE which is the only user of that
function and uses next_initializable_field in there.

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

I'm afraid I'm lost on how to fix the C++11/14 case, the object
being initialized has std::atomic<int> type, which doesn't have any
direct non-static data members, and has std::__atomic_base<int> as base
class that has some field.  The only FIELD_DECL in std::atomic<int>
is DECL_ARTIFICIAL field for the base class, so next_initializable_field
or the hand-written variant thereof doesn't find any initializable fields
for 11/14.  The initializer is initializer list { {1} } and the function
just returns true if it doesn't find any initializable fields, so in the end
we misoptimize it as { {0} }.

2019-10-22  Jakub Jelinek  <jakub@redhat.com>

	PR c++/90947
	* tree.h (type_initializer_zero_p): Remove.
	* tree.c (type_initializer_zero_p): Remove.
cp/
	* cp-tree.h (type_initializer_zero_p): Declare.
	* decl.c (reshape_init_array_1): Formatting fix.
	* tree.c (type_initializer_zero_p): New function.  Moved from
	../tree.c, use next_initializable_field, formatting fix.

--- gcc/tree.h.jj	2019-09-20 12:25:13.737920929 +0200
+++ gcc/tree.h	2019-10-22 10:07:26.411826804 +0200
@@ -4690,12 +4690,6 @@ extern tree first_field (const_tree);
 extern bool initializer_zerop (const_tree, bool * = NULL);
 extern bool initializer_each_zero_or_onep (const_tree);
 
-/* Analogous to initializer_zerop but also examines the type for
-   which the initializer is being used.  Unlike initializer_zerop,
-   considers empty strings to be zero initializers for arrays and
-   non-zero for pointers.  */
-extern bool type_initializer_zero_p (tree, tree);
-
 extern wide_int vector_cst_int_elt (const_tree, unsigned int);
 extern tree vector_cst_elt (const_tree, unsigned int);
 
--- gcc/tree.c.jj	2019-10-19 09:22:14.830893404 +0200
+++ gcc/tree.c	2019-10-22 10:08:36.501748481 +0200
@@ -11396,73 +11396,6 @@ initializer_each_zero_or_onep (const_tre
     }
 }
 
-/* Given an initializer INIT for a TYPE, return true if INIT is zero
-   so that it can be replaced by value initialization.  This function
-   distinguishes betwen empty strings as initializers for arrays and
-   for pointers (which make it return false).  */
-
-bool
-type_initializer_zero_p (tree type, tree init)
-{
-  if (type  == error_mark_node || init == error_mark_node)
-    return false;
-
-  STRIP_NOPS (init);
-
-  if (POINTER_TYPE_P (type))
-    return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
-
-  if (TREE_CODE (init) != CONSTRUCTOR)
-    return initializer_zerop (init);
-
-  if (TREE_CODE (type) == ARRAY_TYPE)
-    {
-      tree elt_type = TREE_TYPE (type);
-      elt_type = TYPE_MAIN_VARIANT (elt_type);
-      if (elt_type == char_type_node)
-	return initializer_zerop (init);
-
-      tree elt_init;
-      unsigned HOST_WIDE_INT i;
-      FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
-	if (!type_initializer_zero_p (elt_type, elt_init))
-	  return false;
-      return true;
-    }
-
-  if (TREE_CODE (type) != RECORD_TYPE)
-    return initializer_zerop (init);
-
-  tree fld = TYPE_FIELDS (type);
-
-  tree fld_init;
-  unsigned HOST_WIDE_INT i;
-  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
-    {
-      /* Advance to the next member, skipping over everything that
-	 canot be initialized (including unnamed bit-fields).  */
-      while (TREE_CODE (fld) != FIELD_DECL
-	     || DECL_ARTIFICIAL (fld)
-	     || (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)))
-	{
-	  fld = DECL_CHAIN (fld);
-	  if (!fld)
-	    return true;
-	  continue;
-	}
-
-      tree fldtype = TREE_TYPE (fld);
-      if (!type_initializer_zero_p (fldtype, fld_init))
-	return false;
-
-      fld = DECL_CHAIN (fld);
-      if (!fld)
-	break;
-    }
-
-  return true;
-}
-
 /* Check if vector VEC consists of all the equal elements and
    that the number of elements corresponds to the type of VEC.
    The function returns first element of the vector
--- gcc/cp/cp-tree.h.jj	2019-10-22 08:15:53.810775827 +0200
+++ gcc/cp/cp-tree.h	2019-10-22 10:10:57.265582861 +0200
@@ -7379,6 +7379,11 @@ extern tree cxx_copy_lang_qualifiers		(c
 
 extern void cxx_print_statistics		(void);
 extern bool maybe_warn_zero_as_null_pointer_constant (tree, location_t);
+/* Analogous to initializer_zerop but also examines the type for
+   which the initializer is being used.  Unlike initializer_zerop,
+   considers empty strings to be zero initializers for arrays and
+   non-zero for pointers.  */
+extern bool type_initializer_zero_p		(tree, tree);
 
 /* in ptree.c */
 extern void cxx_print_xnode			(FILE *, tree, int);
--- gcc/cp/decl.c.jj	2019-10-22 08:57:12.913654620 +0200
+++ gcc/cp/decl.c	2019-10-22 10:13:39.038094034 +0200
@@ -5982,9 +5982,8 @@ reshape_init_array_1 (tree elt_type, tre
       /* Pointers initialized to strings must be treated as non-zero
 	 even if the string is empty.  */
       tree init_type = TREE_TYPE (elt_init);
-      if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)))
-	last_nonzero = index;
-      else if (!type_initializer_zero_p (elt_type, elt_init))
+      if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)
+	  || !type_initializer_zero_p (elt_type, elt_init))
 	last_nonzero = index;
 
       /* This can happen with an invalid initializer (c++/54501).  */
--- gcc/cp/tree.c.jj	2019-10-19 09:22:16.594866462 +0200
+++ gcc/cp/tree.c	2019-10-22 10:10:29.151015397 +0200
@@ -5527,6 +5527,65 @@ maybe_warn_zero_as_null_pointer_constant
   return false;
 }
 \f
+/* Given an initializer INIT for a TYPE, return true if INIT is zero
+   so that it can be replaced by value initialization.  This function
+   distinguishes betwen empty strings as initializers for arrays and
+   for pointers (which make it return false).  */
+
+bool
+type_initializer_zero_p (tree type, tree init)
+{
+  if (type == error_mark_node || init == error_mark_node)
+    return false;
+
+  STRIP_NOPS (init);
+
+  if (POINTER_TYPE_P (type))
+    return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
+
+  if (TREE_CODE (init) != CONSTRUCTOR)
+    return initializer_zerop (init);
+
+  if (TREE_CODE (type) == ARRAY_TYPE)
+    {
+      tree elt_type = TREE_TYPE (type);
+      elt_type = TYPE_MAIN_VARIANT (elt_type);
+      if (elt_type == char_type_node)
+	return initializer_zerop (init);
+
+      tree elt_init;
+      unsigned HOST_WIDE_INT i;
+      FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
+	if (!type_initializer_zero_p (elt_type, elt_init))
+	  return false;
+      return true;
+    }
+
+  if (TREE_CODE (type) != RECORD_TYPE)
+    return initializer_zerop (init);
+
+  tree fld = TYPE_FIELDS (type);
+
+  tree fld_init;
+  unsigned HOST_WIDE_INT i;
+  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
+    {
+      fld = next_initializable_field (fld);
+      if (!fld)
+	return true;
+
+      tree fldtype = TREE_TYPE (fld);
+      if (!type_initializer_zero_p (fldtype, fld_init))
+	return false;
+
+      fld = DECL_CHAIN (fld);
+      if (!fld)
+	break;
+    }
+
+  return true;
+}
+\f
 #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007)
 /* Complain that some language-specific thing hanging off a tree
    node has been accessed improperly.  */

	Jakub

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

* Re: [C++ PATCH] Partial fix for a recent regression (PR c++/90947)
  2019-10-22 15:20 [C++ PATCH] Partial fix for a recent regression (PR c++/90947) Jakub Jelinek
@ 2019-10-29 20:26 ` Jason Merrill
  2019-10-30 23:29   ` [C++ PATCH] Fix " Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2019-10-29 20:26 UTC (permalink / raw)
  To: Jakub Jelinek, Martin Sebor; +Cc: gcc-patches

On 10/22/19 11:03 AM, Jakub Jelinek wrote:
> Hi!
> 
> The following patch is just a partial fix for a regression introduced
> in the PR90947 changes, the testcase is fixed for just C++17/20.
> type_initializer_zero_p has been added to the generic code, supposedly
> because similar initializer_zerop is in generic code too, but that
> means it has a hand-written copy of next_initializable_field which can't
> do exactly what next_initializable_field does.
> 
> The following patch moves it into the C++ FE which is the only user of that
> function and uses next_initializable_field in there.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> I'm afraid I'm lost on how to fix the C++11/14 case, the object
> being initialized has std::atomic<int> type, which doesn't have any
> direct non-static data members, and has std::__atomic_base<int> as base
> class that has some field.  The only FIELD_DECL in std::atomic<int>
> is DECL_ARTIFICIAL field for the base class, so next_initializable_field
> or the hand-written variant thereof doesn't find any initializable fields
> for 11/14.  The initializer is initializer list { {1} } and the function
> just returns true if it doesn't find any initializable fields, so in the end
> we misoptimize it as { {0} }.

I think type_initializer_zero_p should return false if 
CLASSTYPE_NON_AGGREGATE; we can't expect that value-initialization will 
have the intended effect in that case.

> 2019-10-22  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/90947
> 	* tree.h (type_initializer_zero_p): Remove.
> 	* tree.c (type_initializer_zero_p): Remove.
> cp/
> 	* cp-tree.h (type_initializer_zero_p): Declare.
> 	* decl.c (reshape_init_array_1): Formatting fix.
> 	* tree.c (type_initializer_zero_p): New function.  Moved from
> 	../tree.c, use next_initializable_field, formatting fix.
> 
> --- gcc/tree.h.jj	2019-09-20 12:25:13.737920929 +0200
> +++ gcc/tree.h	2019-10-22 10:07:26.411826804 +0200
> @@ -4690,12 +4690,6 @@ extern tree first_field (const_tree);
>   extern bool initializer_zerop (const_tree, bool * = NULL);
>   extern bool initializer_each_zero_or_onep (const_tree);
>   
> -/* Analogous to initializer_zerop but also examines the type for
> -   which the initializer is being used.  Unlike initializer_zerop,
> -   considers empty strings to be zero initializers for arrays and
> -   non-zero for pointers.  */
> -extern bool type_initializer_zero_p (tree, tree);
> -
>   extern wide_int vector_cst_int_elt (const_tree, unsigned int);
>   extern tree vector_cst_elt (const_tree, unsigned int);
>   
> --- gcc/tree.c.jj	2019-10-19 09:22:14.830893404 +0200
> +++ gcc/tree.c	2019-10-22 10:08:36.501748481 +0200
> @@ -11396,73 +11396,6 @@ initializer_each_zero_or_onep (const_tre
>       }
>   }
>   
> -/* Given an initializer INIT for a TYPE, return true if INIT is zero
> -   so that it can be replaced by value initialization.  This function
> -   distinguishes betwen empty strings as initializers for arrays and
> -   for pointers (which make it return false).  */
> -
> -bool
> -type_initializer_zero_p (tree type, tree init)
> -{
> -  if (type  == error_mark_node || init == error_mark_node)
> -    return false;
> -
> -  STRIP_NOPS (init);
> -
> -  if (POINTER_TYPE_P (type))
> -    return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
> -
> -  if (TREE_CODE (init) != CONSTRUCTOR)
> -    return initializer_zerop (init);
> -
> -  if (TREE_CODE (type) == ARRAY_TYPE)
> -    {
> -      tree elt_type = TREE_TYPE (type);
> -      elt_type = TYPE_MAIN_VARIANT (elt_type);
> -      if (elt_type == char_type_node)
> -	return initializer_zerop (init);
> -
> -      tree elt_init;
> -      unsigned HOST_WIDE_INT i;
> -      FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
> -	if (!type_initializer_zero_p (elt_type, elt_init))
> -	  return false;
> -      return true;
> -    }
> -
> -  if (TREE_CODE (type) != RECORD_TYPE)
> -    return initializer_zerop (init);
> -
> -  tree fld = TYPE_FIELDS (type);
> -
> -  tree fld_init;
> -  unsigned HOST_WIDE_INT i;
> -  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
> -    {
> -      /* Advance to the next member, skipping over everything that
> -	 canot be initialized (including unnamed bit-fields).  */
> -      while (TREE_CODE (fld) != FIELD_DECL
> -	     || DECL_ARTIFICIAL (fld)
> -	     || (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)))
> -	{
> -	  fld = DECL_CHAIN (fld);
> -	  if (!fld)
> -	    return true;
> -	  continue;
> -	}
> -
> -      tree fldtype = TREE_TYPE (fld);
> -      if (!type_initializer_zero_p (fldtype, fld_init))
> -	return false;
> -
> -      fld = DECL_CHAIN (fld);
> -      if (!fld)
> -	break;
> -    }
> -
> -  return true;
> -}
> -
>   /* Check if vector VEC consists of all the equal elements and
>      that the number of elements corresponds to the type of VEC.
>      The function returns first element of the vector
> --- gcc/cp/cp-tree.h.jj	2019-10-22 08:15:53.810775827 +0200
> +++ gcc/cp/cp-tree.h	2019-10-22 10:10:57.265582861 +0200
> @@ -7379,6 +7379,11 @@ extern tree cxx_copy_lang_qualifiers		(c
>   
>   extern void cxx_print_statistics		(void);
>   extern bool maybe_warn_zero_as_null_pointer_constant (tree, location_t);
> +/* Analogous to initializer_zerop but also examines the type for
> +   which the initializer is being used.  Unlike initializer_zerop,
> +   considers empty strings to be zero initializers for arrays and
> +   non-zero for pointers.  */
> +extern bool type_initializer_zero_p		(tree, tree);
>   
>   /* in ptree.c */
>   extern void cxx_print_xnode			(FILE *, tree, int);
> --- gcc/cp/decl.c.jj	2019-10-22 08:57:12.913654620 +0200
> +++ gcc/cp/decl.c	2019-10-22 10:13:39.038094034 +0200
> @@ -5982,9 +5982,8 @@ reshape_init_array_1 (tree elt_type, tre
>         /* Pointers initialized to strings must be treated as non-zero
>   	 even if the string is empty.  */
>         tree init_type = TREE_TYPE (elt_init);
> -      if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)))
> -	last_nonzero = index;
> -      else if (!type_initializer_zero_p (elt_type, elt_init))
> +      if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)
> +	  || !type_initializer_zero_p (elt_type, elt_init))
>   	last_nonzero = index;
>   
>         /* This can happen with an invalid initializer (c++/54501).  */
> --- gcc/cp/tree.c.jj	2019-10-19 09:22:16.594866462 +0200
> +++ gcc/cp/tree.c	2019-10-22 10:10:29.151015397 +0200
> @@ -5527,6 +5527,65 @@ maybe_warn_zero_as_null_pointer_constant
>     return false;
>   }
>   \f
> +/* Given an initializer INIT for a TYPE, return true if INIT is zero
> +   so that it can be replaced by value initialization.  This function
> +   distinguishes betwen empty strings as initializers for arrays and
> +   for pointers (which make it return false).  */
> +
> +bool
> +type_initializer_zero_p (tree type, tree init)
> +{
> +  if (type == error_mark_node || init == error_mark_node)
> +    return false;
> +
> +  STRIP_NOPS (init);
> +
> +  if (POINTER_TYPE_P (type))
> +    return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
> +
> +  if (TREE_CODE (init) != CONSTRUCTOR)
> +    return initializer_zerop (init);
> +
> +  if (TREE_CODE (type) == ARRAY_TYPE)
> +    {
> +      tree elt_type = TREE_TYPE (type);
> +      elt_type = TYPE_MAIN_VARIANT (elt_type);
> +      if (elt_type == char_type_node)
> +	return initializer_zerop (init);
> +
> +      tree elt_init;
> +      unsigned HOST_WIDE_INT i;
> +      FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
> +	if (!type_initializer_zero_p (elt_type, elt_init))
  > +	  return false;
> +      return true;
> +    }
> +
> +  if (TREE_CODE (type) != RECORD_TYPE)
> +    return initializer_zerop (init);
> +
> +  tree fld = TYPE_FIELDS (type);
> +
> +  tree fld_init;
> +  unsigned HOST_WIDE_INT i;
> +  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
> +    {
> +      fld = next_initializable_field (fld);
> +      if (!fld)
> +	return true;
> +
> +      tree fldtype = TREE_TYPE (fld);
> +      if (!type_initializer_zero_p (fldtype, fld_init))
> +	return false;
> +
> +      fld = DECL_CHAIN (fld);
> +      if (!fld)
> +	break;
> +    }
> +
> +  return true;
> +}
> +\f
>   #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007)
>   /* Complain that some language-specific thing hanging off a tree
>      node has been accessed improperly.  */
> 
> 	Jakub
> 

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

* [C++ PATCH] Fix a recent regression (PR c++/90947)
  2019-10-29 20:26 ` Jason Merrill
@ 2019-10-30 23:29   ` Jakub Jelinek
  2019-10-31  7:27     ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2019-10-30 23:29 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Martin Sebor, gcc-patches

On Tue, Oct 29, 2019 at 04:26:14PM -0400, Jason Merrill wrote:
> I think type_initializer_zero_p should return false if
> CLASSTYPE_NON_AGGREGATE; we can't expect that value-initialization will have
> the intended effect in that case.

That indeed works for this testcase and doesn't break anything else in the
testsuite.  I've actually used TYPE_NON_AGGREGATE_CLASS because a
RECORD_TYPE might not be always a CLASS_TYPE_P.

Here is what I've bootstrapped/regtested on x86_64-linux and i686-linux, ok
for trunk?

2019-10-30  Jakub Jelinek  <jakub@redhat.com>

	PR c++/90947
	* tree.h (type_initializer_zero_p): Remove.
	* tree.c (type_initializer_zero_p): Remove.
cp/
	* cp-tree.h (type_initializer_zero_p): Declare.
	* decl.c (reshape_init_array_1): Formatting fix.
	* tree.c (type_initializer_zero_p): New function.  Moved from
	../tree.c, use next_initializable_field, formatting fix.  Return
	false for TYPE_NON_AGGREGATE_CLASS types.

--- gcc/tree.h.jj	2019-10-30 08:13:49.017848260 +0100
+++ gcc/tree.h	2019-10-30 08:57:59.457046198 +0100
@@ -4690,12 +4690,6 @@ extern tree first_field (const_tree);
 extern bool initializer_zerop (const_tree, bool * = NULL);
 extern bool initializer_each_zero_or_onep (const_tree);
 
-/* Analogous to initializer_zerop but also examines the type for
-   which the initializer is being used.  Unlike initializer_zerop,
-   considers empty strings to be zero initializers for arrays and
-   non-zero for pointers.  */
-extern bool type_initializer_zero_p (tree, tree);
-
 extern wide_int vector_cst_int_elt (const_tree, unsigned int);
 extern tree vector_cst_elt (const_tree, unsigned int);
 
--- gcc/tree.c.jj	2019-10-30 08:13:48.974848922 +0100
+++ gcc/tree.c	2019-10-30 08:57:59.460046152 +0100
@@ -11396,73 +11396,6 @@ initializer_each_zero_or_onep (const_tre
     }
 }
 
-/* Given an initializer INIT for a TYPE, return true if INIT is zero
-   so that it can be replaced by value initialization.  This function
-   distinguishes betwen empty strings as initializers for arrays and
-   for pointers (which make it return false).  */
-
-bool
-type_initializer_zero_p (tree type, tree init)
-{
-  if (type  == error_mark_node || init == error_mark_node)
-    return false;
-
-  STRIP_NOPS (init);
-
-  if (POINTER_TYPE_P (type))
-    return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
-
-  if (TREE_CODE (init) != CONSTRUCTOR)
-    return initializer_zerop (init);
-
-  if (TREE_CODE (type) == ARRAY_TYPE)
-    {
-      tree elt_type = TREE_TYPE (type);
-      elt_type = TYPE_MAIN_VARIANT (elt_type);
-      if (elt_type == char_type_node)
-	return initializer_zerop (init);
-
-      tree elt_init;
-      unsigned HOST_WIDE_INT i;
-      FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
-	if (!type_initializer_zero_p (elt_type, elt_init))
-	  return false;
-      return true;
-    }
-
-  if (TREE_CODE (type) != RECORD_TYPE)
-    return initializer_zerop (init);
-
-  tree fld = TYPE_FIELDS (type);
-
-  tree fld_init;
-  unsigned HOST_WIDE_INT i;
-  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
-    {
-      /* Advance to the next member, skipping over everything that
-	 canot be initialized (including unnamed bit-fields).  */
-      while (TREE_CODE (fld) != FIELD_DECL
-	     || DECL_ARTIFICIAL (fld)
-	     || (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)))
-	{
-	  fld = DECL_CHAIN (fld);
-	  if (!fld)
-	    return true;
-	  continue;
-	}
-
-      tree fldtype = TREE_TYPE (fld);
-      if (!type_initializer_zero_p (fldtype, fld_init))
-	return false;
-
-      fld = DECL_CHAIN (fld);
-      if (!fld)
-	break;
-    }
-
-  return true;
-}
-
 /* Check if vector VEC consists of all the equal elements and
    that the number of elements corresponds to the type of VEC.
    The function returns first element of the vector
--- gcc/cp/cp-tree.h.jj	2019-10-30 08:13:48.819851308 +0100
+++ gcc/cp/cp-tree.h	2019-10-30 08:57:59.462046121 +0100
@@ -7380,6 +7380,11 @@ extern tree cxx_copy_lang_qualifiers		(c
 
 extern void cxx_print_statistics		(void);
 extern bool maybe_warn_zero_as_null_pointer_constant (tree, location_t);
+/* Analogous to initializer_zerop but also examines the type for
+   which the initializer is being used.  Unlike initializer_zerop,
+   considers empty strings to be zero initializers for arrays and
+   non-zero for pointers.  */
+extern bool type_initializer_zero_p		(tree, tree);
 
 /* in ptree.c */
 extern void cxx_print_xnode			(FILE *, tree, int);
--- gcc/cp/decl.c.jj	2019-10-30 08:13:48.885850291 +0100
+++ gcc/cp/decl.c	2019-10-30 08:57:59.466046060 +0100
@@ -5972,9 +5972,8 @@ reshape_init_array_1 (tree elt_type, tre
       /* Pointers initialized to strings must be treated as non-zero
 	 even if the string is empty.  */
       tree init_type = TREE_TYPE (elt_init);
-      if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)))
-	last_nonzero = index;
-      else if (!type_initializer_zero_p (elt_type, elt_init))
+      if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)
+	  || !type_initializer_zero_p (elt_type, elt_init))
 	last_nonzero = index;
 
       /* This can happen with an invalid initializer (c++/54501).  */
--- gcc/cp/tree.c.jj	2019-10-30 08:13:48.777851956 +0100
+++ gcc/cp/tree.c	2019-10-30 09:17:23.743135729 +0100
@@ -5541,6 +5541,68 @@ maybe_warn_zero_as_null_pointer_constant
   return false;
 }
 \f
+/* Given an initializer INIT for a TYPE, return true if INIT is zero
+   so that it can be replaced by value initialization.  This function
+   distinguishes betwen empty strings as initializers for arrays and
+   for pointers (which make it return false).  */
+
+bool
+type_initializer_zero_p (tree type, tree init)
+{
+  if (type == error_mark_node || init == error_mark_node)
+    return false;
+
+  STRIP_NOPS (init);
+
+  if (POINTER_TYPE_P (type))
+    return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
+
+  if (TREE_CODE (init) != CONSTRUCTOR)
+    return initializer_zerop (init);
+
+  if (TREE_CODE (type) == ARRAY_TYPE)
+    {
+      tree elt_type = TREE_TYPE (type);
+      elt_type = TYPE_MAIN_VARIANT (elt_type);
+      if (elt_type == char_type_node)
+	return initializer_zerop (init);
+
+      tree elt_init;
+      unsigned HOST_WIDE_INT i;
+      FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
+	if (!type_initializer_zero_p (elt_type, elt_init))
+	  return false;
+      return true;
+    }
+
+  if (TREE_CODE (type) != RECORD_TYPE)
+    return initializer_zerop (init);
+
+  if (TYPE_NON_AGGREGATE_CLASS (type))
+    return false;
+
+  tree fld = TYPE_FIELDS (type);
+
+  tree fld_init;
+  unsigned HOST_WIDE_INT i;
+  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
+    {
+      fld = next_initializable_field (fld);
+      if (!fld)
+	return true;
+
+      tree fldtype = TREE_TYPE (fld);
+      if (!type_initializer_zero_p (fldtype, fld_init))
+	return false;
+
+      fld = DECL_CHAIN (fld);
+      if (!fld)
+	break;
+    }
+
+  return true;
+}
+\f
 #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007)
 /* Complain that some language-specific thing hanging off a tree
    node has been accessed improperly.  */
--- gcc/testsuite/g++.dg/init/array54.C.jj	2019-10-30 09:21:12.676608668 +0100
+++ gcc/testsuite/g++.dg/init/array54.C	2019-10-30 09:26:42.813522422 +0100
@@ -0,0 +1,13 @@
+// PR c++/90947
+// { dg-do run { target c++11 } }
+
+#include <atomic>
+
+static std::atomic<int> a[1] { {1} };
+
+int
+main ()
+{
+  if (a[0].load () != 1)
+    __builtin_abort ();
+}


	Jakub

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

* Re: [C++ PATCH] Fix a recent regression (PR c++/90947)
  2019-10-30 23:29   ` [C++ PATCH] Fix " Jakub Jelinek
@ 2019-10-31  7:27     ` Jason Merrill
  0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2019-10-31  7:27 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Martin Sebor, gcc-patches

On 10/30/19 7:29 PM, Jakub Jelinek wrote:
> On Tue, Oct 29, 2019 at 04:26:14PM -0400, Jason Merrill wrote:
>> I think type_initializer_zero_p should return false if
>> CLASSTYPE_NON_AGGREGATE; we can't expect that value-initialization will have
>> the intended effect in that case.
> 
> That indeed works for this testcase and doesn't break anything else in the
> testsuite.  I've actually used TYPE_NON_AGGREGATE_CLASS because a
> RECORD_TYPE might not be always a CLASS_TYPE_P.
> 
> Here is what I've bootstrapped/regtested on x86_64-linux and i686-linux, ok
> for trunk?

OK.

> 2019-10-30  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/90947
> 	* tree.h (type_initializer_zero_p): Remove.
> 	* tree.c (type_initializer_zero_p): Remove.
> cp/
> 	* cp-tree.h (type_initializer_zero_p): Declare.
> 	* decl.c (reshape_init_array_1): Formatting fix.
> 	* tree.c (type_initializer_zero_p): New function.  Moved from
> 	../tree.c, use next_initializable_field, formatting fix.  Return
> 	false for TYPE_NON_AGGREGATE_CLASS types.
> 
> --- gcc/tree.h.jj	2019-10-30 08:13:49.017848260 +0100
> +++ gcc/tree.h	2019-10-30 08:57:59.457046198 +0100
> @@ -4690,12 +4690,6 @@ extern tree first_field (const_tree);
>   extern bool initializer_zerop (const_tree, bool * = NULL);
>   extern bool initializer_each_zero_or_onep (const_tree);
>   
> -/* Analogous to initializer_zerop but also examines the type for
> -   which the initializer is being used.  Unlike initializer_zerop,
> -   considers empty strings to be zero initializers for arrays and
> -   non-zero for pointers.  */
> -extern bool type_initializer_zero_p (tree, tree);
> -
>   extern wide_int vector_cst_int_elt (const_tree, unsigned int);
>   extern tree vector_cst_elt (const_tree, unsigned int);
>   
> --- gcc/tree.c.jj	2019-10-30 08:13:48.974848922 +0100
> +++ gcc/tree.c	2019-10-30 08:57:59.460046152 +0100
> @@ -11396,73 +11396,6 @@ initializer_each_zero_or_onep (const_tre
>       }
>   }
>   
> -/* Given an initializer INIT for a TYPE, return true if INIT is zero
> -   so that it can be replaced by value initialization.  This function
> -   distinguishes betwen empty strings as initializers for arrays and
> -   for pointers (which make it return false).  */
> -
> -bool
> -type_initializer_zero_p (tree type, tree init)
> -{
> -  if (type  == error_mark_node || init == error_mark_node)
> -    return false;
> -
> -  STRIP_NOPS (init);
> -
> -  if (POINTER_TYPE_P (type))
> -    return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
> -
> -  if (TREE_CODE (init) != CONSTRUCTOR)
> -    return initializer_zerop (init);
> -
> -  if (TREE_CODE (type) == ARRAY_TYPE)
> -    {
> -      tree elt_type = TREE_TYPE (type);
> -      elt_type = TYPE_MAIN_VARIANT (elt_type);
> -      if (elt_type == char_type_node)
> -	return initializer_zerop (init);
> -
> -      tree elt_init;
> -      unsigned HOST_WIDE_INT i;
> -      FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
> -	if (!type_initializer_zero_p (elt_type, elt_init))
> -	  return false;
> -      return true;
> -    }
> -
> -  if (TREE_CODE (type) != RECORD_TYPE)
> -    return initializer_zerop (init);
> -
> -  tree fld = TYPE_FIELDS (type);
> -
> -  tree fld_init;
> -  unsigned HOST_WIDE_INT i;
> -  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
> -    {
> -      /* Advance to the next member, skipping over everything that
> -	 canot be initialized (including unnamed bit-fields).  */
> -      while (TREE_CODE (fld) != FIELD_DECL
> -	     || DECL_ARTIFICIAL (fld)
> -	     || (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)))
> -	{
> -	  fld = DECL_CHAIN (fld);
> -	  if (!fld)
> -	    return true;
> -	  continue;
> -	}
> -
> -      tree fldtype = TREE_TYPE (fld);
> -      if (!type_initializer_zero_p (fldtype, fld_init))
> -	return false;
> -
> -      fld = DECL_CHAIN (fld);
> -      if (!fld)
> -	break;
> -    }
> -
> -  return true;
> -}
> -
>   /* Check if vector VEC consists of all the equal elements and
>      that the number of elements corresponds to the type of VEC.
>      The function returns first element of the vector
> --- gcc/cp/cp-tree.h.jj	2019-10-30 08:13:48.819851308 +0100
> +++ gcc/cp/cp-tree.h	2019-10-30 08:57:59.462046121 +0100
> @@ -7380,6 +7380,11 @@ extern tree cxx_copy_lang_qualifiers		(c
>   
>   extern void cxx_print_statistics		(void);
>   extern bool maybe_warn_zero_as_null_pointer_constant (tree, location_t);
> +/* Analogous to initializer_zerop but also examines the type for
> +   which the initializer is being used.  Unlike initializer_zerop,
> +   considers empty strings to be zero initializers for arrays and
> +   non-zero for pointers.  */
> +extern bool type_initializer_zero_p		(tree, tree);
>   
>   /* in ptree.c */
>   extern void cxx_print_xnode			(FILE *, tree, int);
> --- gcc/cp/decl.c.jj	2019-10-30 08:13:48.885850291 +0100
> +++ gcc/cp/decl.c	2019-10-30 08:57:59.466046060 +0100
> @@ -5972,9 +5972,8 @@ reshape_init_array_1 (tree elt_type, tre
>         /* Pointers initialized to strings must be treated as non-zero
>   	 even if the string is empty.  */
>         tree init_type = TREE_TYPE (elt_init);
> -      if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)))
> -	last_nonzero = index;
> -      else if (!type_initializer_zero_p (elt_type, elt_init))
> +      if (POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)
> +	  || !type_initializer_zero_p (elt_type, elt_init))
>   	last_nonzero = index;
>   
>         /* This can happen with an invalid initializer (c++/54501).  */
> --- gcc/cp/tree.c.jj	2019-10-30 08:13:48.777851956 +0100
> +++ gcc/cp/tree.c	2019-10-30 09:17:23.743135729 +0100
> @@ -5541,6 +5541,68 @@ maybe_warn_zero_as_null_pointer_constant
>     return false;
>   }
>   \f
> +/* Given an initializer INIT for a TYPE, return true if INIT is zero
> +   so that it can be replaced by value initialization.  This function
> +   distinguishes betwen empty strings as initializers for arrays and
> +   for pointers (which make it return false).  */
> +
> +bool
> +type_initializer_zero_p (tree type, tree init)
> +{
> +  if (type == error_mark_node || init == error_mark_node)
> +    return false;
> +
> +  STRIP_NOPS (init);
> +
> +  if (POINTER_TYPE_P (type))
> +    return TREE_CODE (init) != STRING_CST && initializer_zerop (init);
> +
> +  if (TREE_CODE (init) != CONSTRUCTOR)
> +    return initializer_zerop (init);
> +
> +  if (TREE_CODE (type) == ARRAY_TYPE)
> +    {
> +      tree elt_type = TREE_TYPE (type);
> +      elt_type = TYPE_MAIN_VARIANT (elt_type);
> +      if (elt_type == char_type_node)
> +	return initializer_zerop (init);
> +
> +      tree elt_init;
> +      unsigned HOST_WIDE_INT i;
> +      FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init)
> +	if (!type_initializer_zero_p (elt_type, elt_init))
> +	  return false;
> +      return true;
> +    }
> +
> +  if (TREE_CODE (type) != RECORD_TYPE)
> +    return initializer_zerop (init);
> +
> +  if (TYPE_NON_AGGREGATE_CLASS (type))
> +    return false;
> +
> +  tree fld = TYPE_FIELDS (type);
> +
> +  tree fld_init;
> +  unsigned HOST_WIDE_INT i;
> +  FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init)
> +    {
> +      fld = next_initializable_field (fld);
> +      if (!fld)
> +	return true;
> +
> +      tree fldtype = TREE_TYPE (fld);
> +      if (!type_initializer_zero_p (fldtype, fld_init))
> +	return false;
> +
> +      fld = DECL_CHAIN (fld);
> +      if (!fld)
> +	break;
> +    }
> +
> +  return true;
> +}
> +\f
>   #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007)
>   /* Complain that some language-specific thing hanging off a tree
>      node has been accessed improperly.  */
> --- gcc/testsuite/g++.dg/init/array54.C.jj	2019-10-30 09:21:12.676608668 +0100
> +++ gcc/testsuite/g++.dg/init/array54.C	2019-10-30 09:26:42.813522422 +0100
> @@ -0,0 +1,13 @@
> +// PR c++/90947
> +// { dg-do run { target c++11 } }
> +
> +#include <atomic>
> +
> +static std::atomic<int> a[1] { {1} };
> +
> +int
> +main ()
> +{
> +  if (a[0].load () != 1)
> +    __builtin_abort ();
> +}
> 
> 
> 	Jakub
> 

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

end of thread, other threads:[~2019-10-31  2:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-22 15:20 [C++ PATCH] Partial fix for a recent regression (PR c++/90947) Jakub Jelinek
2019-10-29 20:26 ` Jason Merrill
2019-10-30 23:29   ` [C++ PATCH] Fix " Jakub Jelinek
2019-10-31  7:27     ` Jason Merrill

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