Fix rs6000 predicates.md use of decl_replaceable_p

Message ID CAGWvnyn+9it7njcqXNudgbVRF3rRMPwOz7FjC4SjxziVsK8EcA@mail.gmail.com
State New
Headers
Series Fix rs6000 predicates.md use of decl_replaceable_p |

Commit Message

David Edelsohn Nov. 18, 2021, 6:37 p.m. UTC
  || DEFAULT_ABI == ABI_ELFv2)
                         && (SYMBOL_REF_EXTERNAL_P (op)
  

Comments

Jan Hubicka Nov. 18, 2021, 7:07 p.m. UTC | #1
> --- a/gcc/config/rs6000/predicates.md
> +++ b/gcc/config/rs6000/predicates.md
> @@ -1086,7 +1086,9 @@ (define_predicate "current_file_function_operand"
>         (match_test "(DEFAULT_ABI != ABI_AIX || SYMBOL_REF_FUNCTION_P (op))
>                     && (SYMBOL_REF_LOCAL_P (op)
>                         || (op == XEXP (DECL_RTL (current_function_decl), 0)
> -                           && !decl_replaceable_p (current_function_decl)))
> +                           && !decl_replaceable_p (current_function_decl,
> +                                                   opt_for_fn
> (current_function_decl,
> +
> flag_semantic_interposition))))

Thanks, missed the use of the predicate here.
However it is not clear to me why one would care about semantic
interposition at this level.  It seems to me that one more cares whether
the symbol must be always resolved locally.  In this case
cgraph_node::get (current_function_decl)->binds_to_current_def_p (cgraph_node::get (current_function_decl))
would give right answer (and work for cases like functions in comdat groups)

Honza
>                     && !((DEFAULT_ABI == ABI_AIX
>                           || DEFAULT_ABI == ABI_ELFv2)
>                          && (SYMBOL_REF_EXTERNAL_P (op)
  
David Edelsohn Nov. 18, 2021, 7:13 p.m. UTC | #2
On Thu, Nov 18, 2021 at 2:07 PM Jan Hubicka <hubicka@kam.mff.cuni.cz> wrote:
>
> > --- a/gcc/config/rs6000/predicates.md
> > +++ b/gcc/config/rs6000/predicates.md
> > @@ -1086,7 +1086,9 @@ (define_predicate "current_file_function_operand"
> >         (match_test "(DEFAULT_ABI != ABI_AIX || SYMBOL_REF_FUNCTION_P (op))
> >                     && (SYMBOL_REF_LOCAL_P (op)
> >                         || (op == XEXP (DECL_RTL (current_function_decl), 0)
> > -                           && !decl_replaceable_p (current_function_decl)))
> > +                           && !decl_replaceable_p (current_function_decl,
> > +                                                   opt_for_fn
> > (current_function_decl,
> > +
> > flag_semantic_interposition))))
>
> Thanks, missed the use of the predicate here.
> However it is not clear to me why one would care about semantic
> interposition at this level.  It seems to me that one more cares whether
> the symbol must be always resolved locally.  In this case
> cgraph_node::get (current_function_decl)->binds_to_current_def_p (cgraph_node::get (current_function_decl))
> would give right answer (and work for cases like functions in comdat groups)

Hi, Honza

I was trying to fix bootstrap as quickly as possible and used the best
example that I could find.  It definitely can be refined.

Thanks for suggesting a better design. I'll test it.

Thanks, David

>
> Honza
> >                     && !((DEFAULT_ABI == ABI_AIX
> >                           || DEFAULT_ABI == ABI_ELFv2)
> >                          && (SYMBOL_REF_EXTERNAL_P (op)
  
Jan Hubicka Nov. 18, 2021, 7:23 p.m. UTC | #3
> On Thu, Nov 18, 2021 at 2:07 PM Jan Hubicka <hubicka@kam.mff.cuni.cz> wrote:
> >
> > > --- a/gcc/config/rs6000/predicates.md
> > > +++ b/gcc/config/rs6000/predicates.md
> > > @@ -1086,7 +1086,9 @@ (define_predicate "current_file_function_operand"
> > >         (match_test "(DEFAULT_ABI != ABI_AIX || SYMBOL_REF_FUNCTION_P (op))
> > >                     && (SYMBOL_REF_LOCAL_P (op)
> > >                         || (op == XEXP (DECL_RTL (current_function_decl), 0)
> > > -                           && !decl_replaceable_p (current_function_decl)))
> > > +                           && !decl_replaceable_p (current_function_decl,
> > > +                                                   opt_for_fn
> > > (current_function_decl,
> > > +
> > > flag_semantic_interposition))))
> >
> > Thanks, missed the use of the predicate here.
> > However it is not clear to me why one would care about semantic
> > interposition at this level.  It seems to me that one more cares whether
> > the symbol must be always resolved locally.  In this case
> > cgraph_node::get (current_function_decl)->binds_to_current_def_p (cgraph_node::get (current_function_decl))
> > would give right answer (and work for cases like functions in comdat groups)
> 
> Hi, Honza
> 
> I was trying to fix bootstrap as quickly as possible and used the best
> example that I could find.  It definitely can be refined.

Sure, having bootstrap working is good - sorry for missing update here.
> 
> Thanks for suggesting a better design. I'll test it.

It really depends what current_file_function_operand means.  If it is
supposed to check that it is a function that is defined in current file
and always will bind to it, then node->binds_to_current_def_p is good
a right thing to test and you don't need to restrict it to current
function decl.  

 node->binds_to_current_def_p (node2)
returns true if refernces from node2 to node will always bind to the
deifnition you are seeing. So
 - node2 is cgraph_node::get (current_function_ecl)
 - node is the symbol OP refers to
   (accessible by cgraph_node::get (SYMBOL_REF_DECL (op)) I think but
    you will likely need to watch for possible NULLs)

Note that with LTO this will rturn true even if node2 is in different
partition (and thus different object file). If you do not want that you
need to additionally check
 && !node->in_other_partition.

Honza
> 
> Thanks, David
> 
> >
> > Honza
> > >                     && !((DEFAULT_ABI == ABI_AIX
> > >                           || DEFAULT_ABI == ABI_ELFv2)
> > >                          && (SYMBOL_REF_EXTERNAL_P (op)
  

Patch

--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -1086,7 +1086,9 @@  (define_predicate "current_file_function_operand"
        (match_test "(DEFAULT_ABI != ABI_AIX || SYMBOL_REF_FUNCTION_P (op))
                    && (SYMBOL_REF_LOCAL_P (op)
                        || (op == XEXP (DECL_RTL (current_function_decl), 0)
-                           && !decl_replaceable_p (current_function_decl)))
+                           && !decl_replaceable_p (current_function_decl,
+                                                   opt_for_fn
(current_function_decl,
+
flag_semantic_interposition))))
                    && !((DEFAULT_ABI == ABI_AIX