On 9/22/20 4:24 PM, Jakub Jelinek wrote:
On Tue, Sep 22, 2020 at 04:11:19PM +0200, Tobias Burnus wrote:
+      while (node->alias_target)
+        {
+          node = node->ultimate_alias_target ();
At least in theory, ultimate_alias_target can look through multiple aliases.

Granted. But we need to handle two things:
* target alias (such as for __attribute__(alias(...))
  for this one, I can walk 'node = node->alias_target'
  here, I need to mark all nodes on the way.
* C++ aliases
  for this one, I used node = node->ultimate_alias_target ()
  here, I only need to mark the last one as only
  that function decl is streamed out

While it might not do that most of the time because this is executed quite
early, I think we have no guarantees it will never do it.
So I'd prefer what you had in the earlier patch, i.e. do the
ultimate_alias_target call + loop to find the ultimate node, and then
in another loop go from the original node (inclusive) up to the ultimate one
(exclusive) and do what you do in this loop now.
Does that make sense?

I am lost. What do I gain by running the loops twice? Initially
the idea was to return NULL_TREE but as you example showed we need
to mark all unmarked ones until to final node.

Thus, we have to mark all – even if the final one is already
'omp declare target'.

But in that case, why can't we do it in a single loop?



If we assume that there is no c++ aliasing, we could do:

          while (node->alias_target)
            {
              // see assumption above: // node = node->ultimate_alias_target ();
              if (!omp_declare_target_fn_p (node->decl)
                  && !lookup_attribute ("omp declare target host",
                                        DECL_ATTRIBUTES (node->decl)))
                {
                  node->offloadable = 1;
                  DECL_ATTRIBUTES (node->decl)
                    = tree_cons (id, NULL_TREE, DECL_ATTRIBUTES (node->decl));
                }
              node = symtab_node::get (node->alias_target);
            }
          // all node->alias_target resolved
          node = node->ultimate_alias_target ();

That would avoid the in-between calling of ultimate_alias_target() but still
calls it if there is no alias_target or for the final alias target.

Is this (really) better?

BTW: When the assumption about the ordering completely changes,
the current __attribute__(alias(…)) testcase will fail; this might
not catch all issues but at least if it completely changes.

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter

Reply via email to