[PATCH] go/100537 - Bootstrap-O3 and bootstrap-debug fail

[Ian Lance Taylor]

On Mon, May 17, 2021 at 1:17 AM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Fri, May 14, 2021 at 11:19 AM guojiufu via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > On 2021-05-14 15:39, guojiufu via Gcc-patches wrote:
> > > On 2021-05-14 15:15, Richard Biener wrote:
> > >> On May 14, 2021 4:52:56 AM GMT+02:00, Jiufu Guo
> > >> <guojiufu@linux.ibm.com> wrote:
> > >>> As discussed in the PR, Richard mentioned the method to
> > >>> figure out which VAR was not set TREE_ADDRESSABLE, and
> > >>> then cause this failure.  It is address_expression which
> > >>> build addr_expr (build_fold_addr_expr_loc), but not set
> > >>> TREE_ADDRESSABLE.
> > >>>
> > >>> I drafted this patch with reference the comments from Richard
> > >>> in this PR, while I'm not quite sure if more thing need to do.
> > >>> So, please have review, thanks!
> > >>>
> > >>> Bootstrap and regtest pass on ppc64le. Is this ok for trunk?
> > >>
> > >> I suggest to use mark_addresssable unless we're sure expr is always an
> > >> entity where TREE_ADDRESSABLE has the desired meaning.
> >
> > Thanks, Richard!
> > You point out the root concern, I'm not sure ;)
> >
> > With looking at code "mark_addresssable" and code around
> > tree-ssa.c:1013,
> > VAR_P, PARM_DECL, and RESULT_DECL are checked before accessing
> > TREE_ADDRESSABLE.
> > So, just wondering if these entities need to be marked as
> > TREE_ADDRESSABLE?
> >
> > diff --git a/gcc/go/go-gcc.cc b/gcc/go/go-gcc.cc
> > index 5d9dbb5d068..85d324a92cc 100644
> > --- a/gcc/go/go-gcc.cc
> > +++ b/gcc/go/go-gcc.cc
> > @@ -1680,6 +1680,11 @@ Gcc_backend::address_expression(Bexpression*
> > bexpr, Location location)
> >     if (expr == error_mark_node)
> >       return this->error_expression();
> >
> > +  if ((VAR_P(expr)
> > +       || TREE_CODE(expr) == PARM_DECL
> > +       || TREE_CODE(expr) == RESULT_DECL)
> > +    TREE_ADDRESSABLE (expr) = 1;
> > +
>
> The root concern is that mark_addressable does
>
>   while (handled_component_p (x))
>     x = TREE_OPERAND (x, 0);
>
> and I do not know the constraints on 'expr' as passed to
> Gcc_backend::address_expression.
>
> I think we need input from Ian here.  Most FEs have their own *_mark_addressable
> function where they also emit diagnostics (guess this is handled in
> the actual Go frontend).
> Since Gcc_backend does lowering to GENERIC using a middle-end is probably OK.

I doubt I understand all the issues here.

In general the Go frontend only takes the addresses of VAR_DECLs or
PARM_DECLs.  It doesn't bother to set TREE_ADDRESSABLE for global
variables for which TREE_STATIC or DECL_EXTERNAL is true.  For local
variables it sets TREE_ADDRESSABLE based on the is_address_taken
parameter to Gcc_backend::local_variable, and similarly for PARM_DECLs
and Gcc_backend::parameter_variable.

The name in the bug report is for a string initializer, which should
be TREE_STATIC == 1 and TREE_PUBLIC == 0.  Perhaps the fix is simply
to set TREE_ADDRESSABLE in Gcc_backend::immutable_struct and
Gcc_backend::implicit_variable.  I can't see how it would hurt to set
TREE_ADDRESSABLE unnecessarily for a TREE_STATIC variable.

But, again, I doubt I understand all the issues here.

Ian