On 10/5/21 10:32, Qing Zhao wrote:
On Oct 5, 2021, at 3:19 AM, Richard Biener <rguent...@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.z...@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.z...@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.z...@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 <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
