Re: [PATCH 04/N] Fix big memory leak in ix86_valid_target_attribute_p
On 11/12/2015 04:58 PM, Ramana Radhakrishnan wrote:
>
>
> On 12/11/15 15:52, Martin Liška wrote:
>> On 11/12/2015 12:29 PM, Richard Biener wrote:
>>> On Thu, Nov 12, 2015 at 11:03 AM, Martin Liška wrote:
Hello.
Following patch was a bit negotiated with Jakub and can save a huge amount
of memory in cases
where target attributes are heavily utilized.
Can bootstrap and survives regression tests on x86_64-linux-pc.
Ready for trunk?
>>>
>>> +static bool opts_obstack_initialized = false;
>>> +
>>> +/* Initialize opts_obstack if not initialized. */
>>> +
>>> +void
>>> +init_opts_obstack (void)
>>> +{
>>> + if (!opts_obstack_initialized)
>>> +{
>>> + opts_obstack_initialized = true;
>>> + gcc_obstack_init (&opts_obstack);
>>>
>>> you can move the static global to function scope.
>>>
>>> Ok with that change.
>>
>> Done and installed as r230264. Final version of the patch is attached.
>>
>>>
>>> Btw, don't other targets need a similar adjustment to their hook?
>>> Grepping shows arm and nios2.
>>
>> nios2 is not the case as it doesn't utilize:
>> init_options_struct (&func_options, NULL);
>>
>> I've been testing patch for aarch64 that is also included in the email.
>
> The change is also needed in config/aarch64/aarch64.c
> (aarch64_option_valid_attribute_p). The attached patch is for arm i.e. 32 bit
> arm.
>
> Ramana
You are right that the change is for arm32, I wrongly wrote aarch64. However if
you read
aarch64_option_valid_attribute_p, there is no init_options_struct
(&func_options, NULL).
So that, I'm going to test on an arm32 machine.
Martin
>
>>
>> Martin
>>
>>>
>>> Thanks,
>>> Richard.
>>>
>>>
Thanks,
Martin
>>
Re: [PATCH 04/N] Fix big memory leak in ix86_valid_target_attribute_p
On 12/11/15 15:52, Martin Liška wrote:
> On 11/12/2015 12:29 PM, Richard Biener wrote:
>> On Thu, Nov 12, 2015 at 11:03 AM, Martin Liška wrote:
>>> Hello.
>>>
>>> Following patch was a bit negotiated with Jakub and can save a huge amount
>>> of memory in cases
>>> where target attributes are heavily utilized.
>>>
>>> Can bootstrap and survives regression tests on x86_64-linux-pc.
>>>
>>> Ready for trunk?
>>
>> +static bool opts_obstack_initialized = false;
>> +
>> +/* Initialize opts_obstack if not initialized. */
>> +
>> +void
>> +init_opts_obstack (void)
>> +{
>> + if (!opts_obstack_initialized)
>> +{
>> + opts_obstack_initialized = true;
>> + gcc_obstack_init (&opts_obstack);
>>
>> you can move the static global to function scope.
>>
>> Ok with that change.
>
> Done and installed as r230264. Final version of the patch is attached.
>
>>
>> Btw, don't other targets need a similar adjustment to their hook?
>> Grepping shows arm and nios2.
>
> nios2 is not the case as it doesn't utilize:
> init_options_struct (&func_options, NULL);
>
> I've been testing patch for aarch64 that is also included in the email.
The change is also needed in config/aarch64/aarch64.c
(aarch64_option_valid_attribute_p). The attached patch is for arm i.e. 32 bit
arm.
Ramana
>
> Martin
>
>>
>> Thanks,
>> Richard.
>>
>>
>>> Thanks,
>>> Martin
>
Re: [PATCH 04/N] Fix big memory leak in ix86_valid_target_attribute_p
On 11/12/2015 12:29 PM, Richard Biener wrote:
> On Thu, Nov 12, 2015 at 11:03 AM, Martin Liška wrote:
>> Hello.
>>
>> Following patch was a bit negotiated with Jakub and can save a huge amount
>> of memory in cases
>> where target attributes are heavily utilized.
>>
>> Can bootstrap and survives regression tests on x86_64-linux-pc.
>>
>> Ready for trunk?
>
> +static bool opts_obstack_initialized = false;
> +
> +/* Initialize opts_obstack if not initialized. */
> +
> +void
> +init_opts_obstack (void)
> +{
> + if (!opts_obstack_initialized)
> +{
> + opts_obstack_initialized = true;
> + gcc_obstack_init (&opts_obstack);
>
> you can move the static global to function scope.
>
> Ok with that change.
Done and installed as r230264. Final version of the patch is attached.
>
> Btw, don't other targets need a similar adjustment to their hook?
> Grepping shows arm and nios2.
nios2 is not the case as it doesn't utilize:
init_options_struct (&func_options, NULL);
I've been testing patch for aarch64 that is also included in the email.
Martin
>
> Thanks,
> Richard.
>
>
>> Thanks,
>> Martin
>From 2a7ed80a489228bbb3c271ca9d0217cca888eeae Mon Sep 17 00:00:00 2001
From: marxin
Date: Wed, 11 Nov 2015 12:52:11 +0100
Subject: [PATCH] Fix big memory leak in ix86_valid_target_attribute_p
gcc/ChangeLog:
2015-11-12 Martin Liska
* config/i386/i386.c (ix86_valid_target_attribute_p):
Finalize options at the of the function.
* gcc.c (driver_get_configure_time_options): Call newly
introduced init_opts_obstack.
* lto-wrapper.c (main): Likewise.
* opts.c (init_opts_obstack): New function.
(init_options_struct): Call newly
introduced init_opts_obstack.
* opts.h (init_options_struct): Declare.
---
gcc/config/i386/i386.c | 2 ++
gcc/gcc.c | 2 +-
gcc/lto-wrapper.c | 2 +-
gcc/opts.c | 16 +++-
gcc/opts.h | 1 +
5 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index b84a11d..1325cf0 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6237,6 +6237,8 @@ ix86_valid_target_attribute_p (tree fndecl,
DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl) = new_optimize;
}
+ finalize_options_struct (&func_options);
+
return ret;
}
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 8bbf5be..87d1979 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -9915,7 +9915,7 @@ driver_get_configure_time_options (void (*cb) (const char *option,
size_t i;
obstack_init (&obstack);
- gcc_obstack_init (&opts_obstack);
+ init_opts_obstack ();
n_switches = 0;
for (i = 0; i < ARRAY_SIZE (option_default_specs); i++)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 20e67ed..b9ac535 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1355,7 +1355,7 @@ main (int argc, char *argv[])
{
const char *p;
- gcc_obstack_init (&opts_obstack);
+ init_opts_obstack ();
p = argv[0] + strlen (argv[0]);
while (p != argv[0] && !IS_DIR_SEPARATOR (p[-1]))
diff --git a/gcc/opts.c b/gcc/opts.c
index 9a3fbb3..930ae43 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -266,6 +266,20 @@ add_comma_separated_to_vector (void **pvec, const char *arg)
*pvec = v;
}
+/* Initialize opts_obstack if not initialized. */
+
+void
+init_opts_obstack (void)
+{
+ static bool opts_obstack_initialized = false;
+
+ if (!opts_obstack_initialized)
+{
+ opts_obstack_initialized = true;
+ gcc_obstack_init (&opts_obstack);
+}
+}
+
/* Initialize OPTS and OPTS_SET before using them in parsing options. */
void
@@ -273,7 +287,7 @@ init_options_struct (struct gcc_options *opts, struct gcc_options *opts_set)
{
size_t num_params = get_num_compiler_params ();
- gcc_obstack_init (&opts_obstack);
+ init_opts_obstack ();
*opts = global_options_init;
diff --git a/gcc/opts.h b/gcc/opts.h
index 38b3837..2eb2d97 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -323,6 +323,7 @@ extern void decode_cmdline_options_to_array (unsigned int argc,
extern void init_options_once (void);
extern void init_options_struct (struct gcc_options *opts,
struct gcc_options *opts_set);
+extern void init_opts_obstack (void);
extern void finalize_options_struct (struct gcc_options *opts);
extern void decode_cmdline_options_to_array_default_mask (unsigned int argc,
const char **argv,
--
2.6.2
>From b2a7dcae8de93db470da0b35162f5538337320ee Mon Sep 17 00:00:00 2001
From: marxin
Date: Thu, 12 Nov 2015 16:41:16 +0100
Subject: [PATCH] Finalize func_options in arm target in
arm_valid_target_attribute_p
gcc/ChangeLog:
2015-11-12 Martin Liska
* config/arm/arm.c (arm_valid_target_attribute_p): Finalize
options struct.
---
gcc/config/arm/arm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index f4ebbc8..941774b 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -29956,6 +29956,8 @@ arm_valid_target_attribute_p (tre
Re: [PATCH 04/N] Fix big memory leak in ix86_valid_target_attribute_p
On 11/12/2015 12:33 PM, Bernd Schmidt wrote:
> On 11/12/2015 12:29 PM, Richard Biener wrote:
>> +static bool opts_obstack_initialized = false;
>> +
>> +/* Initialize opts_obstack if not initialized. */
>> +
>> +void
>> +init_opts_obstack (void)
>> +{
>> + if (!opts_obstack_initialized)
>> +{
>> + opts_obstack_initialized = true;
>> + gcc_obstack_init (&opts_obstack);
>>
>> you can move the static global to function scope.
>
> Also, why bother with it? Why not simply arrange to call the function just
> once at startup?
Hello.
It's called from multiple locations:
$ git grep init_opts_obstack
gcc/gcc.c: init_opts_obstack ();
gcc/lto-wrapper.c: init_opts_obstack ();
gcc/opts.c:init_opts_obstack (void)
gcc/opts.c: init_opts_obstack ();
gcc/opts.h:extern void init_opts_obstack (void);
Maybe there's a common ancestor of all paths, maybe it can be done as a
follow-up.
>
> It's not clear from the submission why this is done and how it relates to the
> i386.c hunk.
Sorry that the hunk isn't explained better, in fast this change is unrelated
and fixed
another memory leak.
Thanks,
Martin
>
>
> Bernd
Re: [PATCH 04/N] Fix big memory leak in ix86_valid_target_attribute_p
On 11/12/2015 12:29 PM, Richard Biener wrote:
+static bool opts_obstack_initialized = false;
+
+/* Initialize opts_obstack if not initialized. */
+
+void
+init_opts_obstack (void)
+{
+ if (!opts_obstack_initialized)
+{
+ opts_obstack_initialized = true;
+ gcc_obstack_init (&opts_obstack);
you can move the static global to function scope.
Also, why bother with it? Why not simply arrange to call the function
just once at startup?
It's not clear from the submission why this is done and how it relates
to the i386.c hunk.
Bernd
Re: [PATCH 04/N] Fix big memory leak in ix86_valid_target_attribute_p
On Thu, Nov 12, 2015 at 11:03 AM, Martin Liška wrote:
> Hello.
>
> Following patch was a bit negotiated with Jakub and can save a huge amount of
> memory in cases
> where target attributes are heavily utilized.
>
> Can bootstrap and survives regression tests on x86_64-linux-pc.
>
> Ready for trunk?
+static bool opts_obstack_initialized = false;
+
+/* Initialize opts_obstack if not initialized. */
+
+void
+init_opts_obstack (void)
+{
+ if (!opts_obstack_initialized)
+{
+ opts_obstack_initialized = true;
+ gcc_obstack_init (&opts_obstack);
you can move the static global to function scope.
Ok with that change.
Btw, don't other targets need a similar adjustment to their hook?
Grepping shows arm and nios2.
Thanks,
Richard.
> Thanks,
> Martin
[PATCH 04/N] Fix big memory leak in ix86_valid_target_attribute_p
Hello.
Following patch was a bit negotiated with Jakub and can save a huge amount of
memory in cases
where target attributes are heavily utilized.
Can bootstrap and survives regression tests on x86_64-linux-pc.
Ready for trunk?
Thanks,
Martin
>From ebb7bd3cf513dc437622868eddbed6c8f725a67c Mon Sep 17 00:00:00 2001
From: marxin
Date: Wed, 11 Nov 2015 12:52:11 +0100
Subject: [PATCH] Fix big memory leak in ix86_valid_target_attribute_p
---
gcc/config/i386/i386.c | 2 ++
gcc/gcc.c | 2 +-
gcc/lto-wrapper.c | 2 +-
gcc/opts-common.c | 1 +
gcc/opts.c | 16 +++-
gcc/opts.h | 1 +
6 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index b84a11d..1325cf0 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -6237,6 +6237,8 @@ ix86_valid_target_attribute_p (tree fndecl,
DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl) = new_optimize;
}
+ finalize_options_struct (&func_options);
+
return ret;
}
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 8bbf5be..87d1979 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -9915,7 +9915,7 @@ driver_get_configure_time_options (void (*cb) (const char *option,
size_t i;
obstack_init (&obstack);
- gcc_obstack_init (&opts_obstack);
+ init_opts_obstack ();
n_switches = 0;
for (i = 0; i < ARRAY_SIZE (option_default_specs); i++)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 20e67ed..b9ac535 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1355,7 +1355,7 @@ main (int argc, char *argv[])
{
const char *p;
- gcc_obstack_init (&opts_obstack);
+ init_opts_obstack ();
p = argv[0] + strlen (argv[0]);
while (p != argv[0] && !IS_DIR_SEPARATOR (p[-1]))
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index d9bf4d4..06e88b5 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -706,6 +706,7 @@ decode_cmdline_option (const char **argv, unsigned int lang_mask,
/* Obstack for option strings. */
struct obstack opts_obstack;
+bool opts_obstack_initialized = false;
/* Like libiberty concat, but allocate using opts_obstack. */
diff --git a/gcc/opts.c b/gcc/opts.c
index 9a3fbb3..527e678 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -266,6 +266,20 @@ add_comma_separated_to_vector (void **pvec, const char *arg)
*pvec = v;
}
+static bool opts_obstack_initialized = false;
+
+/* Initialize opts_obstack if not initialized. */
+
+void
+init_opts_obstack (void)
+{
+ if (!opts_obstack_initialized)
+{
+ opts_obstack_initialized = true;
+ gcc_obstack_init (&opts_obstack);
+}
+}
+
/* Initialize OPTS and OPTS_SET before using them in parsing options. */
void
@@ -273,7 +287,7 @@ init_options_struct (struct gcc_options *opts, struct gcc_options *opts_set)
{
size_t num_params = get_num_compiler_params ();
- gcc_obstack_init (&opts_obstack);
+ init_opts_obstack ();
*opts = global_options_init;
diff --git a/gcc/opts.h b/gcc/opts.h
index 38b3837..2eb2d97 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -323,6 +323,7 @@ extern void decode_cmdline_options_to_array (unsigned int argc,
extern void init_options_once (void);
extern void init_options_struct (struct gcc_options *opts,
struct gcc_options *opts_set);
+extern void init_opts_obstack (void);
extern void finalize_options_struct (struct gcc_options *opts);
extern void decode_cmdline_options_to_array_default_mask (unsigned int argc,
const char **argv,
--
2.6.2
