On Tue, 15 Sep 2026, Tobias Burnus wrote:
> Tobias Burnus wrote:
> > Richard Biener wrote:
> >
> >> But that means we have multiple varpool nodes for the same decl.
> >> Can you varpool->debug () them?
> >
> > [...] Well, it turned out that offload_handle_link_vars is simply called
> > multiple times. In the source code (lto/lto.cc) it is in:
>
> Any final comments about this? - Needing time to ponder about it?
>
> Or should I just keep claiming that it is OpenMP maintaining stuff,
> given that
>
> offload_handle_link_vars ()
>
> is only doing something on (OpenMP) offload-side compiler and then
> also only when the 'omp declare target link' attr is set?
I'm not sure, you pushed some of the changes under discussion, so
I thought this one was off the table. That said, adding new nodes
should be safe, it seems to append at the start. But that's of
course an unwritten property of the current implementation.
Richard.
> Tobias
>
> Namely:
>
> >
> > do_whole_program_analysis (void)
> > {
> > ...
> > /* Find out statics that need to be promoted
> > to globals with hidden visibility because they are accessed from
> > multiple
> > partitions. */
> > lto_promote_cross_file_statics ();
> > offload_handle_link_vars ();
> >
> >
> > And in lto_main it is called once unconditionally and then in
> > the 'if' branch via above function while calling it directly
> > in the 'else' branch:
> >
> >
> > lto_main (void)
> > {
> > ...
> > if (!seen_error ())
> > {
> > offload_handle_link_vars ();
> >
> > /* If WPA is enabled analyze the whole call graph and create an
> > optimization plan. Otherwise, read in all the function
> > bodies and continue with optimization. */
> > if (flag_wpa)
> > do_whole_program_analysis ();
> > else
> > {
> > timevar_start (TV_PHASE_OPT_GEN);
> > materialize_cgraph ();
> > if (!flag_ltrans)
> > {
> > lto_promote_statics_nonwpa ();
> > offload_handle_link_vars ();
> > }
> >
> > * * *
> >
> > For good reasons, I added the call after the first bits of
> > do_whole_program_analysis
> > and in the else branch - which happened in commit
> >
> > r12-7776-g1002a7ace111d7
> > LTO: Fixes for renaming issues with offload/OpenMP [PR104285]
> >
> > That commit handled same-named static variables in multiple translation
> > units - where the original variable was mangled to avoid naming collisions.
> >
> > But seemingly I missed to remove the original call to
> > offload_handle_link_vars
> > and as it didn't break and just wasted resources (walk + build_decl) ...
> >
> > In any case, I do not see any good reason for keeping the old call that is
> > first in the file.
> >
> > * * *
> >
> > This gives the following patch ...
> >
> >
> > Tobias
> >
> > gcc/lto/lto.cc | 35 ++++++++++++++++++++---------------
> > 1 file changed, 20 insertions(+), 15 deletions(-)
> >
> > diff --git a/gcc/lto/lto.cc b/gcc/lto/lto.cc
> > index d22827bbcc0..cf470c73195 100644
> > --- a/gcc/lto/lto.cc
> > +++ b/gcc/lto/lto.cc
> > @@ -482,27 +482,32 @@ static void
> > offload_handle_link_vars (void)
> > {
> > #ifdef ACCEL_COMPILER
> > + size_t i;
> > varpool_node *var;
> > + auto_vec <varpool_node *>temp_link_vars;
> > FOR_EACH_VARIABLE (var)
> > if (lookup_attribute ("omp declare target link",
> > DECL_ATTRIBUTES (var->decl)))
> > - {
> > - if (DECL_HAS_VALUE_EXPR_P (var->decl))
> > - continue;
> > - tree type = build_pointer_type (TREE_TYPE (var->decl));
> > - tree link_ptr_var = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> > + temp_link_vars.safe_push (var);
> > + FOR_EACH_VEC_ELT (temp_link_vars, i, var)
> > + {
> > + /* For some reasons, the same varpool node can appear multiple times.
> > */
> > + if (DECL_HAS_VALUE_EXPR_P (var->decl))
> > + continue;
> > + tree type = build_pointer_type (TREE_TYPE (var->decl));
> > + tree link_ptr_var = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> > clone_function_name (var->decl,
> > "linkptr"),
> > type);
> > - TREE_USED (link_ptr_var) = 1;
> > - TREE_STATIC (link_ptr_var) = 1;
> > - TREE_PUBLIC (link_ptr_var) = TREE_PUBLIC (var->decl);
> > - DECL_ARTIFICIAL (link_ptr_var) = 1;
> > - SET_DECL_ASSEMBLER_NAME (link_ptr_var, DECL_NAME (link_ptr_var));
> > - SET_DECL_VALUE_EXPR (var->decl, build_simple_mem_ref (link_ptr_var));
> > - DECL_HAS_VALUE_EXPR_P (var->decl) = 1;
> > - varpool_node::finalize_decl (link_ptr_var);
> > - varpool_node::get (link_ptr_var)->force_output = var->force_output;
> > - }
> > + TREE_USED (link_ptr_var) = 1;
> > + TREE_STATIC (link_ptr_var) = 1;
> > + TREE_PUBLIC (link_ptr_var) = TREE_PUBLIC (var->decl);
> > + DECL_ARTIFICIAL (link_ptr_var) = 1;
> > + SET_DECL_ASSEMBLER_NAME (link_ptr_var, DECL_NAME (link_ptr_var));
> > + SET_DECL_VALUE_EXPR (var->decl, build_simple_mem_ref (link_ptr_var));
> > + DECL_HAS_VALUE_EXPR_P (var->decl) = 1;
> > + varpool_node::finalize_decl (link_ptr_var);
> > + varpool_node::get (link_ptr_var)->force_output = var->force_output;
> > + }
> > #endif
> > }
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)
