Re: [PATCH] strub: add note on attribute access

[Richard Biener <richard.guenther@gmail.com>]

On Sat, Dec 9, 2023 at 3:09 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Dec  7, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
>
> > Thanks for raising the issue.  Maybe there should be at least a comment
> > there, and perhaps some asserts to check that pointer and reference
> > types don't make to indirect_parms.
>
> Document why attribute access doesn't need the same treatment as fn
> spec, and check that the assumption behind it holds.
>
> Regstrapped on x86_64-linux-gnu.  Ok to install?

OK

>
> for  gcc/ChangeLog
>
>         * ipa-strub.cc (pass_ipa_strub::execute): Check that we don't
>         add indirection to pointer parameters, and document attribute
>         access non-interactions.
> ---
>  gcc/ipa-strub.cc |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
> index 2afb7a455751d..8ec6824e8a802 100644
> --- a/gcc/ipa-strub.cc
> +++ b/gcc/ipa-strub.cc
> @@ -2889,6 +2889,13 @@ pass_ipa_strub::execute (function *)
>                 && (tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (nparm)))
>                     <= 4 * UNITS_PER_WORD))))
>         {
> +         /* No point in indirecting pointer types.  Presumably they
> +            won't ever pass the size-based test above, but check the
> +            assumption here, because getting this wrong would mess
> +            with attribute access and possibly others.  We deal with
> +            fn spec below.  */
> +         gcc_checking_assert (!POINTER_TYPE_P (TREE_TYPE (nparm)));
> +
>           indirect_nparms.add (nparm);
>
>           /* ??? Is there any case in which it is not safe to suggest the parms
> @@ -2976,7 +2983,9 @@ pass_ipa_strub::execute (function *)
>                 }
>             }
>
> -       /* ??? Maybe we could adjust it instead.  */
> +       /* ??? Maybe we could adjust it instead.  Note we don't need
> +          to mess with attribute access: pointer-typed parameters are
> +          not modified, so they can remain unchanged.  */
>         if (drop_fnspec)
>           remove_named_attribute_unsharing ("fn spec",
>                                             &TYPE_ATTRIBUTES (nftype));
>
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive