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. > tree ret = build_fold_addr_expr_loc(location.gcc_location(), expr); > return this->make_expression(ret); > } > > > Or call mark_addressable, and update mark_addressable to avoid NULL > pointer ICE: > The below patch also pass bootstrap-debug. > > diff --git a/gcc/gimple-expr.c b/gcc/gimple-expr.c > index b8c732b632a..f682841391b 100644 > --- a/gcc/gimple-expr.c > +++ b/gcc/gimple-expr.c > @@ -915,6 +915,7 @@ mark_addressable (tree x) > if (TREE_CODE (x) == VAR_DECL > && !DECL_EXTERNAL (x) > && !TREE_STATIC (x) > + && cfun != NULL I'd be OK with this hunk of course. > && cfun->gimple_df != NULL > && cfun->gimple_df->decls_to_pointers != NULL) > { > diff --git a/gcc/go/go-gcc.cc b/gcc/go/go-gcc.cc > index 5d9dbb5d068..fe9dfaf8579 100644 > --- a/gcc/go/go-gcc.cc > +++ b/gcc/go/go-gcc.cc > @@ -1680,6 +1680,7 @@ Gcc_backend::address_expression(Bexpression* > bexpr, Location location) > if (expr == error_mark_node) > return this->error_expression(); > > + mark_addressable(expr); > tree ret = build_fold_addr_expr_loc(location.gcc_location(), expr); > return this->make_expression(ret); > } > > > > > > I notice you mentioned "mark_addresssable" in PR. > > And I had tried yesterday, it cause new ICEs at gimple-expr.c:918 > > below line: > > > > && cfun->gimple_df != NULL > > > > > > > >> > >> Richard. > >> > >>> Jiufu Guo. > >>> > >>> 2021-05-14 Richard Biener <rguent...@suse.de> > >>> Jiufu Guo <guoji...@linux.ibm.com> > >>> > >>> PR go/100537 > >>> * go-gcc.cc > >>> (Gcc_backend::address_expression): Set TREE_ADDRESSABLE. > >>> > >>> --- > >>> gcc/go/go-gcc.cc | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/gcc/go/go-gcc.cc b/gcc/go/go-gcc.cc > >>> index 5d9dbb5d068..8ed20a3b479 100644 > >>> --- a/gcc/go/go-gcc.cc > >>> +++ b/gcc/go/go-gcc.cc > >>> @@ -1680,6 +1680,7 @@ Gcc_backend::address_expression(Bexpression* > >>> bexpr, Location location) > >>> if (expr == error_mark_node) > >>> return this->error_expression(); > >>> > >>> + TREE_ADDRESSABLE (expr) = 1; > >>> tree ret = build_fold_addr_expr_loc(location.gcc_location(), expr); > >>> return this->make_expression(ret); > >>> }