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 > > >> <guoji...@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
