> On Sep 30, 2021, at 2:31 PM, Jason Merrill <ja...@redhat.com> wrote: > > On 9/30/21 11:42, Qing Zhao wrote: >>> 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. > > The problem with this test is not whether the 'this' proxy is marked > READONLY, the problem is that you're trying to initialize lambda capture > proxies at all; the lambda capture objects were already initialized when > forming the closure object. So this test currently aborts with > -ftrivial-auto-var-init=zero because you "initialize" the i capture field to > 0 after it was previously initialized to 42: > > int main() > { > int i = 42; > auto l = [=]() mutable { return i; }; > if (l() != i) > __builtin_abort (); > } > > I believe the same issue applies to the proxy variables in coroutines that > work much like lambdas. So, how should the middle end determine that a variable is “proxy variable”? Have all “proxy variables” been initialized by C++ FE already? > > You can't just assume that a VAR_DECL with DECL_VALUE_EXPR is uninitialized.
So, all the VAR_DECLs with DECL_VALUE_EXPR (except the ones created by “gimplify_decl_expr”) are initialized by FE already? > > Since there's already VLA handling in gimplify_decl_expr, you could remember > whether you added DECL_VALUE_EXPR in that function, and only then do the > initialization. Yes, if we can guarantee that all the VAR_DECLs with DECL_VALUE_EXPR created from FEs have been initialized already by FE, we can fix this issue as this way. thanks. Qing > > Jason >
