On Sep 30, 2021, at 1:54 AM, Richard Biener <rguent...@suse.de> wrote:
On Thu, 30 Sep 2021, Jason Merrill wrote: On 9/29/21 17:30, Qing Zhao wrote: Hi, PR102359 (ICE gimplification failed since r12-3433-ga25e0b5e6ac8a77a) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102359 Is due to -ftrivial-auto-var-init adding initialization for READONLY variable “this” in the following routine: (t.cpp.005t.original) ======= ;; Function A::foo()::<lambda()> (null) ;; enabled by -tree-original { const struct A * const this [value-expr: &__closure->__this]; const struct A * const this [value-expr: &__closure->__this]; return <retval> = (double) ((const struct A *) this)->a; } ======= However, in the above routine, “this” is NOT marked as READONLY, but its value-expr "&__closure->__this” is marked as READONLY. There are two major issues: 1. In the routine “is_var_need_auto_init”, we should exclude “decl” that is marked as READONLY; 2. In the C++ FE, “this” should be marked as READONLY. The idea solution will be: 1. Fix “is_var_need_auto_init” to exclude TREE_READONLY (decl); 2. Fix C++ FE to mark “this” as TREE_READONLY (decl)==true; Not sure whether it’s hard for C++ FE to fix the 2nd issue or not? In the case it’s not a quick fix in C++FE, I proposed the following fix in middle end: Let me know your comments or suggestions on this. Thanks a lot for the help. I'd think is_var_need_auto_init should be false for any variable with DECL_HAS_VALUE_EXPR_P, as they aren't really variables, just ways of naming objects that are initialized elsewhere. IIRC handing variables with DECL_HAS_VALUE_EXPR_P is necessary to auto-init VLAs, otherwise I tend to agree - would we handle those when we see a DECL_EXPR then? The current implementation is: gimplify_decl_expr: For each DECL_EXPR “decl” If (VAR_P (decl) && !DECL_EXTERNAL (decl)) { if (is_vla (decl)) gimplify_vla_decl (decl, …); /* existing handling: create a VALUE_EXPR for this vla decl*/ … if (has_explicit_init (decl)) { …; /* existing handling. */ } else if (is_var_need_auto_init (decl)) /*. New code. */ { gimple_add_init_for_auto_var (….); /* new code. */ ... } } Since the “DECL_VALUE_EXPR (decl)” is NOT a DECL_EXPR, it will not be scanned and added initialization. if we do not add initialization for a decl that has DECL_VALUE_EXPR, then the “DECL_VALUE_EXPR (decl)” will not be added an initialization either. We will miss adding initializations for these decls. So, I think that the current implementation is correct. And if C++ FE will not mark “this” as READONLY, only mark DECL_VALUE_EXPR(this) as READONLY, the proposed fix is correct too. Let me know your opinion on this. Thanks. Qing Qing ============================== From 0a5982cd61bc4610655d3df00ae8d2fbcb3c8e9b Mon Sep 17 00:00:00 2001 From: Qing Zhao <qing.z...@oracle.com> Date: Wed, 29 Sep 2021 20:49:59 +0000 Subject: [PATCH] Fix PR102359 --- gcc/gimplify.c | 15 +++++++++++++++ gcc/testsuite/g++.dg/pr102359.C | 13 +++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 gcc/testsuite/g++.dg/pr102359.C diff --git a/gcc/gimplify.c b/gcc/gimplify.c index 1067113b1639..a2587869b35d 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -1819,12 +1819,27 @@ 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 READONLY. + This is to workaround a C++ FE bug that only mark the value_expr of "this" + as readonly but does not mark "this" as readonly. + C++ FE should fix this issue before replacing this routine with + TREE_READONLY (decl). */ + +static bool +is_decl_readonly (tree decl) +{ + return (TREE_READONLY (decl) + || (DECL_HAS_VALUE_EXPR_P (decl) + && TREE_READONLY (DECL_VALUE_EXPR (decl)))); +} + /* Return true if the DECL need to be automaticly initialized by the compiler. */ static bool is_var_need_auto_init (tree decl) { if (auto_var_p (decl) + && !is_decl_readonly (decl) && (flag_auto_var_init > AUTO_INIT_UNINITIALIZED) && (!lookup_attribute ("uninitialized", DECL_ATTRIBUTES (decl))) && !is_empty_type (TREE_TYPE (decl))) diff --git a/gcc/testsuite/g++.dg/pr102359.C b/gcc/testsuite/g++.dg/pr102359.C new file mode 100644 index 000000000000..da643cde7bed --- /dev/null +++ b/gcc/testsuite/g++.dg/pr102359.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()(); -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
