inline: do not inline when no_profile_instrument_function is different

Hello.

Similarly to e.g. sanitizer attributes, we sould prevent inlining when one function
is marked as not instrumented. We should do that with -fprofile-generate only.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

Adds test-case for:
	PR gcov-profile/80223

gcc/ChangeLog:

	* ipa-inline.c (can_inline_edge_p): Similarly to sanitizer
	options, do not inline when no_profile_instrument_function
	attributes are different.

gcc/testsuite/ChangeLog:

	* gcc.dg/no_profile_instrument_function-attr-2.c: New test.
---
  gcc/ipa-inline.c                                  | 10 ++++++++++
  .../no_profile_instrument_function-attr-2.c       | 15 +++++++++++++++
  2 files changed, 25 insertions(+)
  create mode 100644 gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-2.c

> Hello.
> 
> Similarly to e.g. sanitizer attributes, we sould prevent inlining when one function
> is marked as not instrumented. We should do that with -fprofile-generate only.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> Adds test-case for:
> 	PR gcov-profile/80223
> 
> gcc/ChangeLog:
> 
> 	* ipa-inline.c (can_inline_edge_p): Similarly to sanitizer
> 	options, do not inline when no_profile_instrument_function
> 	attributes are different.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/no_profile_instrument_function-attr-2.c: New test.
> ---
>  gcc/ipa-inline.c                                  | 10 ++++++++++
>  .../no_profile_instrument_function-attr-2.c       | 15 +++++++++++++++
>  2 files changed, 25 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-2.c
> 
> diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
> index 9d896beb2ac..786ab2e7f7f 100644
> --- a/gcc/ipa-inline.c
> +++ b/gcc/ipa-inline.c
> @@ -396,6 +396,16 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
>        e->inline_failed = CIF_SANITIZE_ATTRIBUTE_MISMATCH;
>        inlinable = false;
>      }
> +  else if (profile_arc_flag
> +	   && lookup_attribute ("no_profile_instrument_function",
> +				DECL_ATTRIBUTES (caller->decl))
> +	      != lookup_attribute ("no_profile_instrument_function",
> +				   DECL_ATTRIBUTES (callee->decl)))
> +    {
> +      e->inline_failed = CIF_UNSPECIFIED;
> +      inlinable = false;
> +    }

Is there reason to prevent the inlining once instrumentation is done?
I think you can just block it for early inliner.

Honza
> +
>    if (!inlinable && report)
>      report_inline_failed_reason (e);
>    return inlinable;
> diff --git a/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-2.c b/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-2.c
> new file mode 100644
> index 00000000000..586962a1c76
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/no_profile_instrument_function-attr-2.c
> @@ -0,0 +1,15 @@
> +/* { dg-require-effective-target global_constructor } */
> +/* { dg-options "-O2 -fprofile-generate -fprofile-update=single -fdump-tree-optimized" } */
> +
> +__attribute__ ((no_profile_instrument_function))
> +int foo()
> +{
> +  return 0;
> +}
> +
> +int bar()
> +{
> +  return foo();
> +}
> +
> +/* { dg-final { scan-tree-dump-times " = foo \\(\\)" 1 "optimized"} } */
> -- 
> 2.32.0
>

On 6/23/21 2:38 PM, Jan Hubicka wrote:
> Is there reason to prevent the inlining once instrumentation is done?

No ;)

> I think you can just block it for early inliner.

Sure. Do you have a handy predicate function that tells if einliner is done?

Thanks,
Martin

On Wed, Jun 23, 2021 at 6:15 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 6/23/21 2:38 PM, Jan Hubicka wrote:
> > Is there reason to prevent the inlining once instrumentation is done?
>
> No ;)

Here's another case that coincidentally came up yesterday. How should
these attributes behave in the case of __attribute__((flatten)) on the
caller?

Another case which is curious is what happens when the callee has
__attribute__((always_inline)) and there's a conflict?

In LLVM, the current behavior is:
"__attribute__((no_stack_protector)) and
__attribute__((no_profile_instrument_function)) will prevent inline
substitution when the attributes do not match between caller and
callee, unless the callee was attributed with
__attribute__((always_inline)) or the caller was attributed with
__attribute__((flatten))."

We have some overzealous users of always_inline and flatten in
Android. They are currently playing whack-a-mole with
no_stack_protector.  I'm not sure yet how we can better help them self
diagnose, or whether we should consider a change in policy.

I'm also not sure whether GCC's einliner corresponds with
always_inline or not necessarily?
--
Thanks,
~Nick Desaulniers

On 6/23/21 3:15 PM, Martin Liška wrote:
> On 6/23/21 2:38 PM, Jan Hubicka wrote:
>> Is there reason to prevent the inlining once instrumentation is done?
> 
> No ;)
> 
>> I think you can just block it for early inliner.
> 
> Sure. Do you have a handy predicate function that tells if einliner is done?
> 
> Thanks,
> Martin

Hello.

There's updated version of that patch that blocks inlining only during einline IPA pass.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

On 8/18/2021 2:38 AM, Martin Liška wrote:
> On 6/23/21 3:15 PM, Martin Liška wrote:
>> On 6/23/21 2:38 PM, Jan Hubicka wrote:
>>> Is there reason to prevent the inlining once instrumentation is done?
>>
>> No ;)
>>
>>> I think you can just block it for early inliner.
>>
>> Sure. Do you have a handy predicate function that tells if einliner 
>> is done?
>>
>> Thanks,
>> Martin
>
> Hello.
>
> There's updated version of that patch that blocks inlining only during 
> einline IPA pass.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> 0001-inline-do-not-einline-when-no_profile_instrument_fun.patch
>
>  From e2adaff9ed92bcc380e1368418da5ad2801fef4e Mon Sep 17 00:00:00 2001
> From: Martin Liska <mliska@suse.cz>
> Date: Tue, 22 Jun 2021 10:09:01 +0200
> Subject: [PATCH] inline: do not einline when no_profile_instrument_function is
>   different
>
> 	PR gcov-profile/80223
>
> gcc/ChangeLog:
>
> 	* ipa-inline.c (can_inline_edge_p): Similarly to sanitizer
> 	options, do not inline when no_profile_instrument_function
> 	attributes are different in early inliner. It's fine to inline
> 	it after PGO instrumentation.
OK
jeff