* [patch][middle-end/PR102359]Not add initialization for DECL_VALUE_EXPR variables with -ftrivial-auto-var-init
@ 2021-10-05 4:52 Qing Zhao
2021-10-05 8:19 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Qing Zhao @ 2021-10-05 4:52 UTC (permalink / raw)
To: Richard Biener; +Cc: jason, gcc-patches Nick Alcock via
Hi,
This is the patch to fix this issue based on our discussion.
I have tested it on aarch64 with bootstrap and regtests. X86 bootstrap was done, regtests is ongoing.
Okay for trunk?
Thanks.
Qing
======================
From d349ef0145512efe7f9af2c6bbd01f636475bce3 Mon Sep 17 00:00:00 2001
From: qing zhao <qing.zhao@oracle.com>
Date: Mon, 4 Oct 2021 15:26:03 -0700
Subject: [PATCH] middle-end/102359 Not add initialization for variables that
have been initialized by FEs.
C++ FE creates proxy variables, which have associated DECL_VALUE_EXPR
and have been initialized by FE. For such auto variable, we should not
add initialization when -ftrivial-auto-var-init presents.
gcc/ChangeLog:
2021-10-04 qing zhao <qing.zhao@oracle.com>
* gimplify.c (is_decl_init_by_fe): New function.
(gimplify_decl_expr): Not add initialization for an auto variable
when it has been initialized by frontend.
gcc/testsuite/ChangeLog:
2021-10-04 qing zhao <qing.zhao@oracle.com>
* g++.dg/pr102359_1.C: New test.
* g++.dg/pr102359_2.C: New test.
---
gcc/gimplify.c | 21 ++++++++++++++++++++-
gcc/testsuite/g++.dg/pr102359_1.C | 13 +++++++++++++
gcc/testsuite/g++.dg/pr102359_2.C | 13 +++++++++++++
3 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/pr102359_1.C
create mode 100644 gcc/testsuite/g++.dg/pr102359_2.C
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index b27776a..d6865ad 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1819,6 +1819,19 @@ gimple_add_padding_init_for_auto_var (tree decl, bool is_vla,
gimplify_seq_add_stmt (seq_p, call);
}
+/* Return true if the DECL is initialized by FE.
+ If the VAR_DECL has DECL_VALUE_EXPR that was created by FE (usually C++FE),
+ it's a proxy varaible, and FE already initializd the DECL_VALUE_EXPR of it.
+*/
+static bool
+is_decl_init_by_fe (tree decl, bool is_created_by_fe)
+{
+ if (DECL_HAS_VALUE_EXPR_P (decl)
+ && is_created_by_fe)
+ return true;
+ return false;
+}
+
/* Return true if the DECL need to be automaticly initialized by the
compiler. */
static bool
@@ -1871,8 +1884,13 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
if (VAR_P (decl) && !DECL_EXTERNAL (decl))
{
tree init = DECL_INITIAL (decl);
+ bool is_value_expr_created_by_fe = false;
bool is_vla = false;
+ /* Check whether a decl has FE created VALUE_EXPR here BEFORE
+ gimplify_vla_decl creates VALUE_EXPR for vla decl. */
+ is_value_expr_created_by_fe = DECL_HAS_VALUE_EXPR_P (decl);
+
poly_uint64 size;
if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), &size)
|| (!TREE_STATIC (decl)
@@ -1934,7 +1952,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
/* When there is no explicit initializer, if the user requested,
We should insert an artifical initializer for this automatic
variable. */
- else if (is_var_need_auto_init (decl))
+ else if (is_var_need_auto_init (decl)
+ && !is_decl_init_by_fe (decl, is_value_expr_created_by_fe))
{
gimple_add_init_for_auto_var (decl,
flag_auto_var_init,
diff --git a/gcc/testsuite/g++.dg/pr102359_1.C b/gcc/testsuite/g++.dg/pr102359_1.C
new file mode 100644
index 0000000..da643cd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr102359_1.C
@@ -0,0 +1,13 @@
+/* PR middle-end/102359 ICE gimplification failed since
+ r12-3433-ga25e0b5e6ac8a77a. */
+/* { dg-do compile } */
+/* { dg-options "-ftrivial-auto-var-init=zero" } */
+/* { dg-require-effective-target c++17 } */
+
+struct A {
+ double a = 111;
+ auto foo() {
+ return [*this] { return a; };
+ }
+};
+int X = A{}.foo()();
diff --git a/gcc/testsuite/g++.dg/pr102359_2.C b/gcc/testsuite/g++.dg/pr102359_2.C
new file mode 100644
index 0000000..d026d72
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr102359_2.C
@@ -0,0 +1,13 @@
+/* PR middle-end/102359 ICE gimplification failed since
+ r12-3433-ga25e0b5e6ac8a77a. */
+/* { dg-do run} */
+/* { dg-options "-ftrivial-auto-var-init=zero" } */
+/* { dg-require-effective-target c++17 } */
+
+int main()
+{
+ int i = 42;
+ auto l = [=]() mutable { return i; };
+ if (l() != i)
+ __builtin_abort ();
+}
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch][middle-end/PR102359]Not add initialization for DECL_VALUE_EXPR variables with -ftrivial-auto-var-init
2021-10-05 4:52 [patch][middle-end/PR102359]Not add initialization for DECL_VALUE_EXPR variables with -ftrivial-auto-var-init Qing Zhao
@ 2021-10-05 8:19 ` Richard Biener
2021-10-05 14:32 ` Qing Zhao
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2021-10-05 8:19 UTC (permalink / raw)
To: Qing Zhao; +Cc: jason, gcc-patches Nick Alcock via
On Tue, 5 Oct 2021, Qing Zhao wrote:
> Hi,
>
> This is the patch to fix this issue based on our discussion.
>
> I have tested it on aarch64 with bootstrap and regtests. X86 bootstrap was done, regtests is ongoing.
>
> Okay for trunk?
>
> Thanks.
>
> Qing
>
> ======================
> From d349ef0145512efe7f9af2c6bbd01f636475bce3 Mon Sep 17 00:00:00 2001
> From: qing zhao <qing.zhao@oracle.com>
> Date: Mon, 4 Oct 2021 15:26:03 -0700
> Subject: [PATCH] middle-end/102359 Not add initialization for variables that
> have been initialized by FEs.
>
> C++ FE creates proxy variables, which have associated DECL_VALUE_EXPR
> and have been initialized by FE. For such auto variable, we should not
> add initialization when -ftrivial-auto-var-init presents.
>
> gcc/ChangeLog:
>
> 2021-10-04 qing zhao <qing.zhao@oracle.com>
>
> * gimplify.c (is_decl_init_by_fe): New function.
> (gimplify_decl_expr): Not add initialization for an auto variable
> when it has been initialized by frontend.
>
> gcc/testsuite/ChangeLog:
>
> 2021-10-04 qing zhao <qing.zhao@oracle.com>
>
> * g++.dg/pr102359_1.C: New test.
> * g++.dg/pr102359_2.C: New test.
> ---
> gcc/gimplify.c | 21 ++++++++++++++++++++-
> gcc/testsuite/g++.dg/pr102359_1.C | 13 +++++++++++++
> gcc/testsuite/g++.dg/pr102359_2.C | 13 +++++++++++++
> 3 files changed, 46 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/pr102359_1.C
> create mode 100644 gcc/testsuite/g++.dg/pr102359_2.C
>
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index b27776a..d6865ad 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -1819,6 +1819,19 @@ gimple_add_padding_init_for_auto_var (tree decl, bool is_vla,
> gimplify_seq_add_stmt (seq_p, call);
> }
>
> +/* Return true if the DECL is initialized by FE.
> + If the VAR_DECL has DECL_VALUE_EXPR that was created by FE (usually C++FE),
> + it's a proxy varaible, and FE already initializd the DECL_VALUE_EXPR of it.
> +*/
> +static bool
> +is_decl_init_by_fe (tree decl, bool is_created_by_fe)
> +{
> + if (DECL_HAS_VALUE_EXPR_P (decl)
> + && is_created_by_fe)
> + return true;
> + return false;
> +}
> +
> /* Return true if the DECL need to be automaticly initialized by the
> compiler. */
> static bool
> @@ -1871,8 +1884,13 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
> if (VAR_P (decl) && !DECL_EXTERNAL (decl))
> {
> tree init = DECL_INITIAL (decl);
> + bool is_value_expr_created_by_fe = false;
no need for the = false, it's always initialized below.
> bool is_vla = false;
>
> + /* Check whether a decl has FE created VALUE_EXPR here BEFORE
> + gimplify_vla_decl creates VALUE_EXPR for vla decl. */
> + is_value_expr_created_by_fe = DECL_HAS_VALUE_EXPR_P (decl);
That looks a bit weird when looking at ...
> +
> poly_uint64 size;
> if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), &size)
> || (!TREE_STATIC (decl)
> @@ -1934,7 +1952,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
> /* When there is no explicit initializer, if the user requested,
> We should insert an artifical initializer for this automatic
> variable. */
> - else if (is_var_need_auto_init (decl))
> + else if (is_var_need_auto_init (decl)
> + && !is_decl_init_by_fe (decl, is_value_expr_created_by_fe))
... which just expands to
if (DECL_HAS_VALUE_EXPR_P (decl) && DECL_HAS_VALUE_EXPR_P (decl))
can you please name 'is_value_expr_created_by_fe' as
'decl_had_value_expr_p' and check && !decl_had_value_expr_p here?
So sth like
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index b27776af7c8..9013f385f13 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1872,6 +1872,7 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
{
tree init = DECL_INITIAL (decl);
bool is_vla = false;
+ bool decl_had_value_expr_p = DECL_HAS_VALUE_EXPR_P (decl);
poly_uint64 size;
if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), &size)
@@ -1934,7 +1935,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
/* When there is no explicit initializer, if the user requested,
We should insert an artifical initializer for this automatic
variable. */
- else if (is_var_need_auto_init (decl))
+ else if (is_var_need_auto_init (decl)
+ && !decl_had_value_expr_p)
{
gimple_add_init_for_auto_var (decl,
flag_auto_var_init,
OK with that change.
Thanks,
Richard.
> {
> gimple_add_init_for_auto_var (decl,
> flag_auto_var_init,
> diff --git a/gcc/testsuite/g++.dg/pr102359_1.C b/gcc/testsuite/g++.dg/pr102359_1.C
> new file mode 100644
> index 0000000..da643cd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr102359_1.C
> @@ -0,0 +1,13 @@
> +/* PR middle-end/102359 ICE gimplification failed since
> + r12-3433-ga25e0b5e6ac8a77a. */
> +/* { dg-do compile } */
> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
> +/* { dg-require-effective-target c++17 } */
> +
> +struct A {
> + double a = 111;
> + auto foo() {
> + return [*this] { return a; };
> + }
> +};
> +int X = A{}.foo()();
> diff --git a/gcc/testsuite/g++.dg/pr102359_2.C b/gcc/testsuite/g++.dg/pr102359_2.C
> new file mode 100644
> index 0000000..d026d72
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/pr102359_2.C
> @@ -0,0 +1,13 @@
> +/* PR middle-end/102359 ICE gimplification failed since
> + r12-3433-ga25e0b5e6ac8a77a. */
> +/* { dg-do run} */
> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
> +/* { dg-require-effective-target c++17 } */
> +
> +int main()
> +{
> + int i = 42;
> + auto l = [=]() mutable { return i; };
> + if (l() != i)
> + __builtin_abort ();
> +}
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch][middle-end/PR102359]Not add initialization for DECL_VALUE_EXPR variables with -ftrivial-auto-var-init
2021-10-05 8:19 ` Richard Biener
@ 2021-10-05 14:32 ` Qing Zhao
2021-10-05 15:36 ` Qing Zhao
2021-10-05 18:30 ` Jason Merrill
0 siblings, 2 replies; 6+ messages in thread
From: Qing Zhao @ 2021-10-05 14:32 UTC (permalink / raw)
To: Richard Biener; +Cc: jason, gcc-patches Nick Alcock via
> On Oct 5, 2021, at 3:19 AM, Richard Biener <rguenther@suse.de> wrote:
>
> On Tue, 5 Oct 2021, Qing Zhao wrote:
>
>> Hi,
>>
>> This is the patch to fix this issue based on our discussion.
>>
>> I have tested it on aarch64 with bootstrap and regtests. X86 bootstrap was done, regtests is ongoing.
>>
>> Okay for trunk?
>>
>> Thanks.
>>
>> Qing
>>
>> ======================
>> From d349ef0145512efe7f9af2c6bbd01f636475bce3 Mon Sep 17 00:00:00 2001
>> From: qing zhao <qing.zhao@oracle.com>
>> Date: Mon, 4 Oct 2021 15:26:03 -0700
>> Subject: [PATCH] middle-end/102359 Not add initialization for variables that
>> have been initialized by FEs.
>>
>> C++ FE creates proxy variables, which have associated DECL_VALUE_EXPR
>> and have been initialized by FE. For such auto variable, we should not
>> add initialization when -ftrivial-auto-var-init presents.
>>
>> gcc/ChangeLog:
>>
>> 2021-10-04 qing zhao <qing.zhao@oracle.com>
>>
>> * gimplify.c (is_decl_init_by_fe): New function.
>> (gimplify_decl_expr): Not add initialization for an auto variable
>> when it has been initialized by frontend.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2021-10-04 qing zhao <qing.zhao@oracle.com>
>>
>> * g++.dg/pr102359_1.C: New test.
>> * g++.dg/pr102359_2.C: New test.
>> ---
>> gcc/gimplify.c | 21 ++++++++++++++++++++-
>> gcc/testsuite/g++.dg/pr102359_1.C | 13 +++++++++++++
>> gcc/testsuite/g++.dg/pr102359_2.C | 13 +++++++++++++
>> 3 files changed, 46 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/g++.dg/pr102359_1.C
>> create mode 100644 gcc/testsuite/g++.dg/pr102359_2.C
>>
>> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
>> index b27776a..d6865ad 100644
>> --- a/gcc/gimplify.c
>> +++ b/gcc/gimplify.c
>> @@ -1819,6 +1819,19 @@ gimple_add_padding_init_for_auto_var (tree decl, bool is_vla,
>> gimplify_seq_add_stmt (seq_p, call);
>> }
>>
>> +/* Return true if the DECL is initialized by FE.
>> + If the VAR_DECL has DECL_VALUE_EXPR that was created by FE (usually C++FE),
>> + it's a proxy varaible, and FE already initializd the DECL_VALUE_EXPR of it.
>> +*/
>> +static bool
>> +is_decl_init_by_fe (tree decl, bool is_created_by_fe)
>> +{
>> + if (DECL_HAS_VALUE_EXPR_P (decl)
>> + && is_created_by_fe)
>> + return true;
>> + return false;
>> +}
>> +
>> /* Return true if the DECL need to be automaticly initialized by the
>> compiler. */
>> static bool
>> @@ -1871,8 +1884,13 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
>> if (VAR_P (decl) && !DECL_EXTERNAL (decl))
>> {
>> tree init = DECL_INITIAL (decl);
>> + bool is_value_expr_created_by_fe = false;
>
> no need for the = false, it's always initialized below.
>
>> bool is_vla = false;
>>
>> + /* Check whether a decl has FE created VALUE_EXPR here BEFORE
>> + gimplify_vla_decl creates VALUE_EXPR for vla decl. */
>> + is_value_expr_created_by_fe = DECL_HAS_VALUE_EXPR_P (decl);
>
> That looks a bit weird when looking at ...
>
>> +
>> poly_uint64 size;
>> if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), &size)
>> || (!TREE_STATIC (decl)
>> @@ -1934,7 +1952,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
>> /* When there is no explicit initializer, if the user requested,
>> We should insert an artifical initializer for this automatic
>> variable. */
>> - else if (is_var_need_auto_init (decl))
>> + else if (is_var_need_auto_init (decl)
>> + && !is_decl_init_by_fe (decl, is_value_expr_created_by_fe))
>
> ... which just expands to
>
> if (DECL_HAS_VALUE_EXPR_P (decl) && DECL_HAS_VALUE_EXPR_P (decl))
>
> can you please name 'is_value_expr_created_by_fe' as
> 'decl_had_value_expr_p' and check && !decl_had_value_expr_p here?
> So sth like
I can do this -:) I agree that the change will make the code simpler.
However, my major concern with this change is: later when people look at this change, they might ask:
Why we should not initialize a variable with VALUE_EXPR? And whether the variable whose VALUE_EXPR
was created by “gimplify_vla_decl” should be excluded?
My new function and comments were all for this purpose.
If I go with this change, at least we should add some comments to explain this as following, what do you think?
>
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index b27776af7c8..9013f385f13 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -1872,6 +1872,7 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
> {
> tree init = DECL_INITIAL (decl);
> bool is_vla = false;
+ /* Check whether a decl has FE created VALUE_EXPR here BEFORE
+ gimplify_vla_decl creates VALUE_EXPR for vla decl. */
> + bool decl_had_value_expr_p = DECL_HAS_VALUE_EXPR_P (decl);
>
> poly_uint64 size;
> if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), &size)
> @@ -1934,7 +1935,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
> /* When there is no explicit initializer, if the user requested,
> We should insert an artifical initializer for this automatic
> variable. */
> - else if (is_var_need_auto_init (decl))
> + else if (is_var_need_auto_init (decl)
+ /* If the VAR_DECL has DECL_VALUE_EXPR that was created by FE (usually C++FE),
+ it's a proxy varaible, and FE already initializd the DECL_VALUE_EXPR of it. */
> + && !decl_had_value_expr_p)
> {
> gimple_add_init_for_auto_var (decl,
> flag_auto_var_init,
>
> OK with that change.
thanks.
Qing
>
> Thanks,
> Richard.
>
>
>> {
>> gimple_add_init_for_auto_var (decl,
>> flag_auto_var_init,
>> diff --git a/gcc/testsuite/g++.dg/pr102359_1.C b/gcc/testsuite/g++.dg/pr102359_1.C
>> new file mode 100644
>> index 0000000..da643cd
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/pr102359_1.C
>> @@ -0,0 +1,13 @@
>> +/* PR middle-end/102359 ICE gimplification failed since
>> + r12-3433-ga25e0b5e6ac8a77a. */
>> +/* { dg-do compile } */
>> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
>> +/* { dg-require-effective-target c++17 } */
>> +
>> +struct A {
>> + double a = 111;
>> + auto foo() {
>> + return [*this] { return a; };
>> + }
>> +};
>> +int X = A{}.foo()();
>> diff --git a/gcc/testsuite/g++.dg/pr102359_2.C b/gcc/testsuite/g++.dg/pr102359_2.C
>> new file mode 100644
>> index 0000000..d026d72
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/pr102359_2.C
>> @@ -0,0 +1,13 @@
>> +/* PR middle-end/102359 ICE gimplification failed since
>> + r12-3433-ga25e0b5e6ac8a77a. */
>> +/* { dg-do run} */
>> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
>> +/* { dg-require-effective-target c++17 } */
>> +
>> +int main()
>> +{
>> + int i = 42;
>> + auto l = [=]() mutable { return i; };
>> + if (l() != i)
>> + __builtin_abort ();
>> +}
>>
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch][middle-end/PR102359]Not add initialization for DECL_VALUE_EXPR variables with -ftrivial-auto-var-init
2021-10-05 14:32 ` Qing Zhao
@ 2021-10-05 15:36 ` Qing Zhao
2021-10-05 18:30 ` Jason Merrill
1 sibling, 0 replies; 6+ messages in thread
From: Qing Zhao @ 2021-10-05 15:36 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches Nick Alcock via
This is the updated patch, I will push it after done with testing.
Let me know if you see any issue.
Thanks.
Qing.
============================
From 1f07c20ecdc9a015369c8bb472ebbd3bcaabdf17 Mon Sep 17 00:00:00 2001
From: qing zhao <qing.zhao@oracle.com>
Date: Tue, 5 Oct 2021 08:10:02 -0700
Subject: [PATCH] middle-end/102359 Not add initialization for variables that
have been initialized by FEs.
C++ FE creates proxy variables, which have associated DECL_VALUE_EXPR
and have been initialized by FE. For such auto variable, we should not
add initialization when -ftrivial-auto-var-init presents.
gcc/ChangeLog:
2021-10-05 qing zhao <qing.zhao@oracle.com>
* gimplify.c (gimplify_decl_expr): Not add initialization for an
auto variable when it has been initialized by frontend.
gcc/testsuite/ChangeLog:
2021-10-05 qing zhao <qing.zhao@oracle.com>
* g++.dg/pr102359_1.C: New test.
* g++.dg/pr102359_2.C: New test.
---
gcc/gimplify.c | 9 ++++++++-
gcc/testsuite/g++.dg/pr102359_1.C | 13 +++++++++++++
gcc/testsuite/g++.dg/pr102359_2.C | 13 +++++++++++++
3 files changed, 34 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/pr102359_1.C
create mode 100644 gcc/testsuite/g++.dg/pr102359_2.C
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index b27776a..d96fca4 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1872,6 +1872,9 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
{
tree init = DECL_INITIAL (decl);
bool is_vla = false;
+ /* Check whether a decl has FE created VALUE_EXPR here BEFORE
+ gimplify_vla_decl creates VALUE_EXPR for a vla decl. */
+ bool decl_had_value_expr_p = DECL_HAS_VALUE_EXPR_P (decl);
poly_uint64 size;
if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), &size)
@@ -1934,7 +1937,11 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
/* When there is no explicit initializer, if the user requested,
We should insert an artifical initializer for this automatic
variable. */
- else if (is_var_need_auto_init (decl))
+ else if (is_var_need_auto_init (decl)
+ /* If the decl has VALUE_EXPR that was created by FE (usually
+ C++FE), it's a proxy varaible, and FE already initialized
+ the VALUE_EXPR of it, we should not initialize it here. */
+ && !decl_had_value_expr_p)
{
gimple_add_init_for_auto_var (decl,
flag_auto_var_init,
diff --git a/gcc/testsuite/g++.dg/pr102359_1.C b/gcc/testsuite/g++.dg/pr102359_1.C
new file mode 100644
index 0000000..da643cd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr102359_1.C
@@ -0,0 +1,13 @@
+/* PR middle-end/102359 ICE gimplification failed since
+ r12-3433-ga25e0b5e6ac8a77a. */
+/* { dg-do compile } */
+/* { dg-options "-ftrivial-auto-var-init=zero" } */
+/* { dg-require-effective-target c++17 } */
+
+struct A {
+ double a = 111;
+ auto foo() {
+ return [*this] { return a; };
+ }
+};
+int X = A{}.foo()();
diff --git a/gcc/testsuite/g++.dg/pr102359_2.C b/gcc/testsuite/g++.dg/pr102359_2.C
new file mode 100644
index 0000000..d026d72
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr102359_2.C
@@ -0,0 +1,13 @@
+/* PR middle-end/102359 ICE gimplification failed since
+ r12-3433-ga25e0b5e6ac8a77a. */
+/* { dg-do run} */
+/* { dg-options "-ftrivial-auto-var-init=zero" } */
+/* { dg-require-effective-target c++17 } */
+
+int main()
+{
+ int i = 42;
+ auto l = [=]() mutable { return i; };
+ if (l() != i)
+ __builtin_abort ();
+}
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch][middle-end/PR102359]Not add initialization for DECL_VALUE_EXPR variables with -ftrivial-auto-var-init
2021-10-05 14:32 ` Qing Zhao
2021-10-05 15:36 ` Qing Zhao
@ 2021-10-05 18:30 ` Jason Merrill
2021-10-05 22:39 ` Qing Zhao
1 sibling, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2021-10-05 18:30 UTC (permalink / raw)
To: Qing Zhao, Richard Biener; +Cc: gcc-patches Nick Alcock via
On 10/5/21 10:32, Qing Zhao wrote:
>
>
>> On Oct 5, 2021, at 3:19 AM, Richard Biener <rguenther@suse.de> wrote:
>>
>> On Tue, 5 Oct 2021, Qing Zhao wrote:
>>
>>> Hi,
>>>
>>> This is the patch to fix this issue based on our discussion.
>>>
>>> I have tested it on aarch64 with bootstrap and regtests. X86 bootstrap was done, regtests is ongoing.
>>>
>>> Okay for trunk?
>>>
>>> Thanks.
>>>
>>> Qing
>>>
>>> ======================
>>> From d349ef0145512efe7f9af2c6bbd01f636475bce3 Mon Sep 17 00:00:00 2001
>>> From: qing zhao <qing.zhao@oracle.com>
>>> Date: Mon, 4 Oct 2021 15:26:03 -0700
>>> Subject: [PATCH] middle-end/102359 Not add initialization for variables that
>>> have been initialized by FEs.
>>>
>>> C++ FE creates proxy variables, which have associated DECL_VALUE_EXPR
>>> and have been initialized by FE. For such auto variable, we should not
>>> add initialization when -ftrivial-auto-var-init presents.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2021-10-04 qing zhao <qing.zhao@oracle.com>
>>>
>>> * gimplify.c (is_decl_init_by_fe): New function.
>>> (gimplify_decl_expr): Not add initialization for an auto variable
>>> when it has been initialized by frontend.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2021-10-04 qing zhao <qing.zhao@oracle.com>
>>>
>>> * g++.dg/pr102359_1.C: New test.
>>> * g++.dg/pr102359_2.C: New test.
>>> ---
>>> gcc/gimplify.c | 21 ++++++++++++++++++++-
>>> gcc/testsuite/g++.dg/pr102359_1.C | 13 +++++++++++++
>>> gcc/testsuite/g++.dg/pr102359_2.C | 13 +++++++++++++
>>> 3 files changed, 46 insertions(+), 1 deletion(-)
>>> create mode 100644 gcc/testsuite/g++.dg/pr102359_1.C
>>> create mode 100644 gcc/testsuite/g++.dg/pr102359_2.C
>>>
>>> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
>>> index b27776a..d6865ad 100644
>>> --- a/gcc/gimplify.c
>>> +++ b/gcc/gimplify.c
>>> @@ -1819,6 +1819,19 @@ gimple_add_padding_init_for_auto_var (tree decl, bool is_vla,
>>> gimplify_seq_add_stmt (seq_p, call);
>>> }
>>>
>>> +/* Return true if the DECL is initialized by FE.
>>> + If the VAR_DECL has DECL_VALUE_EXPR that was created by FE (usually C++FE),
>>> + it's a proxy varaible, and FE already initializd the DECL_VALUE_EXPR of it.
>>> +*/
>>> +static bool
>>> +is_decl_init_by_fe (tree decl, bool is_created_by_fe)
>>> +{
>>> + if (DECL_HAS_VALUE_EXPR_P (decl)
>>> + && is_created_by_fe)
>>> + return true;
>>> + return false;
>>> +}
>>> +
>>> /* Return true if the DECL need to be automaticly initialized by the
>>> compiler. */
>>> static bool
>>> @@ -1871,8 +1884,13 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
>>> if (VAR_P (decl) && !DECL_EXTERNAL (decl))
>>> {
>>> tree init = DECL_INITIAL (decl);
>>> + bool is_value_expr_created_by_fe = false;
>>
>> no need for the = false, it's always initialized below.
>>
>>> bool is_vla = false;
>>>
>>> + /* Check whether a decl has FE created VALUE_EXPR here BEFORE
>>> + gimplify_vla_decl creates VALUE_EXPR for vla decl. */
>>> + is_value_expr_created_by_fe = DECL_HAS_VALUE_EXPR_P (decl);
>>
>> That looks a bit weird when looking at ...
>>
>>> +
>>> poly_uint64 size;
>>> if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), &size)
>>> || (!TREE_STATIC (decl)
>>> @@ -1934,7 +1952,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
>>> /* When there is no explicit initializer, if the user requested,
>>> We should insert an artifical initializer for this automatic
>>> variable. */
>>> - else if (is_var_need_auto_init (decl))
>>> + else if (is_var_need_auto_init (decl)
>>> + && !is_decl_init_by_fe (decl, is_value_expr_created_by_fe))
>>
>> ... which just expands to
>>
>> if (DECL_HAS_VALUE_EXPR_P (decl) && DECL_HAS_VALUE_EXPR_P (decl))
>>
>> can you please name 'is_value_expr_created_by_fe' as
>> 'decl_had_value_expr_p' and check && !decl_had_value_expr_p here?
>> So sth like
>
> I can do this -:) I agree that the change will make the code simpler.
>
> However, my major concern with this change is: later when people look at this change, they might ask:
> Why we should not initialize a variable with VALUE_EXPR? And whether the variable whose VALUE_EXPR
> was created by “gimplify_vla_decl” should be excluded?
>
> My new function and comments were all for this purpose.
>
> If I go with this change, at least we should add some comments to explain this as following, what do you think?
>>
>> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
>> index b27776af7c8..9013f385f13 100644
>> --- a/gcc/gimplify.c
>> +++ b/gcc/gimplify.c
>> @@ -1872,6 +1872,7 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
>> {
>> tree init = DECL_INITIAL (decl);
>> bool is_vla = false;
>
> + /* Check whether a decl has FE created VALUE_EXPR here BEFORE
> + gimplify_vla_decl creates VALUE_EXPR for vla decl. */
>> + bool decl_had_value_expr_p = DECL_HAS_VALUE_EXPR_P (decl);
>>
>> poly_uint64 size;
>> if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), &size)
>> @@ -1934,7 +1935,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
>> /* When there is no explicit initializer, if the user requested,
>> We should insert an artifical initializer for this automatic
>> variable. */
>> - else if (is_var_need_auto_init (decl))
>> + else if (is_var_need_auto_init (decl)
>
> + /* If the VAR_DECL has DECL_VALUE_EXPR that was created by FE (usually C++FE),
> + it's a proxy varaible, and FE already initializd the DECL_VALUE_EXPR of it. */
I'd suggest merging this comment into the comment on the variable
declaration.
>> + && !decl_had_value_expr_p)
>> {
>> gimple_add_init_for_auto_var (decl,
>> flag_auto_var_init,
>>
>> OK with that change.
>
> thanks.
>
> Qing
>>
>> Thanks,
>> Richard.
>>
>>
>>> {
>>> gimple_add_init_for_auto_var (decl,
>>> flag_auto_var_init,
>>> diff --git a/gcc/testsuite/g++.dg/pr102359_1.C b/gcc/testsuite/g++.dg/pr102359_1.C
>>> new file mode 100644
>>> index 0000000..da643cd
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/pr102359_1.C
>>> @@ -0,0 +1,13 @@
>>> +/* PR middle-end/102359 ICE gimplification failed since
>>> + r12-3433-ga25e0b5e6ac8a77a. */
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
>>> +/* { dg-require-effective-target c++17 } */
>>> +
>>> +struct A {
>>> + double a = 111;
>>> + auto foo() {
>>> + return [*this] { return a; };
>>> + }
>>> +};
>>> +int X = A{}.foo()();
>>> diff --git a/gcc/testsuite/g++.dg/pr102359_2.C b/gcc/testsuite/g++.dg/pr102359_2.C
>>> new file mode 100644
>>> index 0000000..d026d72
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/pr102359_2.C
>>> @@ -0,0 +1,13 @@
>>> +/* PR middle-end/102359 ICE gimplification failed since
>>> + r12-3433-ga25e0b5e6ac8a77a. */
>>> +/* { dg-do run} */
>>> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
>>> +/* { dg-require-effective-target c++17 } */
>>> +
>>> +int main()
>>> +{
>>> + int i = 42;
>>> + auto l = [=]() mutable { return i; };
>>> + if (l() != i)
>>> + __builtin_abort ();
>>> +}
>>>
>>
>> --
>> Richard Biener <rguenther@suse.de>
>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch][middle-end/PR102359]Not add initialization for DECL_VALUE_EXPR variables with -ftrivial-auto-var-init
2021-10-05 18:30 ` Jason Merrill
@ 2021-10-05 22:39 ` Qing Zhao
0 siblings, 0 replies; 6+ messages in thread
From: Qing Zhao @ 2021-10-05 22:39 UTC (permalink / raw)
To: Jason Merrill; +Cc: Richard Biener, gcc-patches Nick Alcock via
> On Oct 5, 2021, at 1:30 PM, Jason Merrill <jason@redhat.com> wrote:
>
>>> 'decl_had_value_expr_p' and check && !decl_had_value_expr_p here?
>>> So sth like
>> I can do this -:) I agree that the change will make the code simpler.
>> However, my major concern with this change is: later when people look at this change, they might ask:
>> Why we should not initialize a variable with VALUE_EXPR? And whether the variable whose VALUE_EXPR
>> was created by “gimplify_vla_decl” should be excluded?
>> My new function and comments were all for this purpose.
>> If I go with this change, at least we should add some comments to explain this as following, what do you think?
>>>
>>> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
>>> index b27776af7c8..9013f385f13 100644
>>> --- a/gcc/gimplify.c
>>> +++ b/gcc/gimplify.c
>>> @@ -1872,6 +1872,7 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
>>> {
>>> tree init = DECL_INITIAL (decl);
>>> bool is_vla = false;
>> + /* Check whether a decl has FE created VALUE_EXPR here BEFORE
>> + gimplify_vla_decl creates VALUE_EXPR for vla decl. */
>>> + bool decl_had_value_expr_p = DECL_HAS_VALUE_EXPR_P (decl);
>>>
>>> poly_uint64 size;
>>> if (!poly_int_tree_p (DECL_SIZE_UNIT (decl), &size)
>>> @@ -1934,7 +1935,8 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
>>> /* When there is no explicit initializer, if the user requested,
>>> We should insert an artifical initializer for this automatic
>>> variable. */
>>> - else if (is_var_need_auto_init (decl))
>>> + else if (is_var_need_auto_init (decl)
>> + /* If the VAR_DECL has DECL_VALUE_EXPR that was created by FE (usually C++FE),
>> + it's a proxy varaible, and FE already initializd the DECL_VALUE_EXPR of it. */
>
> I'd suggest merging this comment into the comment on the variable declaration.
Okay, will merge these two comments into one and put it on the variable declaration.
thanks.
Qing
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-05 22:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 4:52 [patch][middle-end/PR102359]Not add initialization for DECL_VALUE_EXPR variables with -ftrivial-auto-var-init Qing Zhao
2021-10-05 8:19 ` Richard Biener
2021-10-05 14:32 ` Qing Zhao
2021-10-05 15:36 ` Qing Zhao
2021-10-05 18:30 ` Jason Merrill
2021-10-05 22:39 ` Qing Zhao
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).