diff mbox

C++ PATCH for c++/53650 (memory-hog with large array)

Message ID 50E5B670.6070607@redhat.com
State New
Headers show

Commit Message

Jason Merrill Jan. 3, 2013, 4:48 p.m. UTC 
My patch for c++/48370 changed initialization of an array from a 
brace-enclosed initializer list so that even when the initializer is 
omitted, we were filling out the CONSTRUCTOR with the appropriate 
value-initialization semantics.  For a large array, this causes the 
internal representation to be a lot larger than iterating over the 
array, initializing all the elements with the same code.  The 48370 fix 
only really needs to do that for array initializers which might contain 
temporaries that need to have their lifetimes extended, so this patch 
limits the earlier change to element types that could be affected.

The build_vec_init hunk is to avoid regressing on c++/35602 now that 
we're using build_vec_init again on that testcase.

Tested x86_64-pc-linux-gnu, applying to trunk for now.

Comments

Jakub Jelinek Jan. 3, 2013, 4:54 p.m. UTC | #1 
On Thu, Jan 03, 2013 at 11:48:48AM -0500, Jason Merrill wrote:
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/init/array34.C
> @@ -0,0 +1,13 @@
> +// PR c++/53650
> +// We should loop over array inits if they don't involve temporaries
> +// that need extending.
> +// { dg-final { scan-assembler-times "_ZN5ClassC1Ev" 1 } }
> +
> +struct Class {
> +  Class();
> +};
> +
> +int main() {
> +  Class table [10] = {};
> +  return 0;
> +}

Won't the test fail on weirdo targets that need some extern directives
for the external symbols?
I'd say safer would be to scan the gimple dump instead...

	Jakub
Eric Botcazou Jan. 23, 2013, 8:10 a.m. UTC | #2 
> Tested x86_64-pc-linux-gnu, applying to trunk for now.

This was put yesterday on the 4.7 branch and broke the build:

../../src/gcc/cp/call.c: In function 'type_has_extended_temps':
../../src/gcc/cp/call.c:8866: error: 'for' loop initial declaration used 
outside C99 mode
make[3]: *** [cp/call.o] Error 1
Eric Botcazou Jan. 23, 2013, 9:28 a.m. UTC | #3 
> Tested x86_64-pc-linux-gnu, applying to trunk for now.

This was put yesterday on the 4.7 branch and broke the build:

../../src/gcc/cp/call.c: In function 'type_has_extended_temps':
../../src/gcc/cp/call.c:8866: error: 'for' loop initial declaration used 
outside C99 mode
make[3]: *** [cp/call.o] Error 1
Jason Merrill Jan. 23, 2013, 2:13 p.m. UTC | #4 
Thanks; I've already fixed it.

Jason
diff mbox

Patch

commit 8174b0f1a41d8c05f1a72868062faaa2229f15f4
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Jan 2 20:37:21 2013 -0500

    	PR c++/53650
    	* call.c (type_has_extended_temps): New.
    	* cp-tree.h: Declare it.
    	* decl.c (check_initializer): Use build_aggr_init for arrays
    	if it is false.
    	* init.c (build_vec_init): Avoid mixed signed/unsigned arithmetic.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index ad39637..1466c4b 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -9234,6 +9234,28 @@  extend_ref_init_temps (tree decl, tree init, vec<tree, va_gc> **cleanups)
   return init;
 }
 
+/* Returns true iff an initializer for TYPE could contain temporaries that
+   need to be extended because they are bound to references or
+   std::initializer_list.  */
+
+bool
+type_has_extended_temps (tree type)
+{
+  type = strip_array_types (type);
+  if (TREE_CODE (type) == REFERENCE_TYPE)
+    return true;
+  if (CLASS_TYPE_P (type))
+    {
+      if (is_std_init_list (type))
+	return true;
+      for (tree f = next_initializable_field (TYPE_FIELDS (type));
+	   f; f = next_initializable_field (DECL_CHAIN (f)))
+	if (type_has_extended_temps (TREE_TYPE (f)))
+	  return true;
+    }
+  return false;
+}
+
 /* Returns true iff TYPE is some variant of std::initializer_list.  */
 
 bool
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 465fa0f..810df7d 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -4952,6 +4952,7 @@  extern tree initialize_reference		(tree, tree, int,
 						 tsubst_flags_t);
 extern tree extend_ref_init_temps		(tree, tree, vec<tree, va_gc>**);
 extern tree make_temporary_var_for_ref_to_temp	(tree, tree);
+extern bool type_has_extended_temps		(tree);
 extern tree strip_top_quals			(tree);
 extern bool reference_related_p			(tree, tree);
 extern tree perform_implicit_conversion		(tree, tree, tsubst_flags_t);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 52ceefc..5c268b1 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -5657,7 +5657,9 @@  check_initializer (tree decl, tree init, int flags, vec<tree, va_gc> **cleanups)
       if ((type_build_ctor_call (type) || CLASS_TYPE_P (type))
 	  && !(flags & LOOKUP_ALREADY_DIGESTED)
 	  && !(init && BRACE_ENCLOSED_INITIALIZER_P (init)
-	       && CP_AGGREGATE_TYPE_P (type)))
+	       && CP_AGGREGATE_TYPE_P (type)
+	       && (CLASS_TYPE_P (type)
+		   || type_has_extended_temps (type))))
 	{
 	  init_code = build_aggr_init_full_exprs (decl, init, flags);
 
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 6edc0a5..2ee2473 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -3637,7 +3637,9 @@  build_vec_init (tree base, tree maxindex, tree init,
       if (TREE_CODE (type) == ARRAY_TYPE)
 	m = cp_build_binary_op (input_location,
 				MULT_EXPR, m,
-				array_type_nelts_total (type),
+				/* Avoid mixing signed and unsigned.  */
+				convert (TREE_TYPE (m),
+					 array_type_nelts_total (type)),
 				complain);
 
       finish_cleanup_try_block (try_block);
diff --git a/gcc/testsuite/g++.dg/init/array34.C b/gcc/testsuite/g++.dg/init/array34.C
new file mode 100644
index 0000000..c5f608b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/array34.C
@@ -0,0 +1,13 @@ 
+// PR c++/53650
+// We should loop over array inits if they don't involve temporaries
+// that need extending.
+// { dg-final { scan-assembler-times "_ZN5ClassC1Ev" 1 } }
+
+struct Class {
+  Class();
+};
+
+int main() {
+  Class table [10] = {};
+  return 0;
+}