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?

> 
> > 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)

Reply via email to