diff mbox series

[1/2] compiler_attributes.h: define __no_profile, add to noinstr

Message ID 20210618233023.1360185-2-ndesaulniers@google.com (mailing list archive)
State New, archived
Headers show
Series no_profile fn attr and Kconfig for GCOV+PGO | expand

Commit Message

Nick Desaulniers June 18, 2021, 11:30 p.m. UTC
noinstr implies that we would like the compiler to avoid instrumenting a
function.  Add support for the compiler attribute no_profile to
compiler_attributes.h, then add __no_profile to the definition of
noinstr.

Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Link: https://lore.kernel.org/lkml/20210614162018.GD68749@worktop.programming.kicks-ass.net/
Link: https://reviews.llvm.org/D104475
Link: https://reviews.llvm.org/D104257
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 include/linux/compiler_attributes.h | 12 ++++++++++++
 include/linux/compiler_types.h      |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Miguel Ojeda June 19, 2021, 11:26 a.m. UTC | #1
On Sat, Jun 19, 2021 at 1:30 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> +/*
> + * Optional: only supported since clang >= 13
> + *      gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
> + *    clang: https://clang.llvm.org/docs/AttributeReference.html#no_profile
> + */

I am not sure if it is best or not to have the GCC link in order to be
consistent with the rest of the links (they are for the docs only). Do
we know if GCC going to implement it soon?

Otherwise, it looks good to me.

Cheers,
Miguel
Miguel Ojeda June 19, 2021, 11:32 a.m. UTC | #2
On Sat, Jun 19, 2021 at 1:26 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> I am not sure if it is best or not to have the GCC link in order to be
> consistent with the rest of the links (they are for the docs only). Do
> we know if GCC going to implement it soon?

i.e. if GCC does not implement it yet we use elsewhere this kind of
marker instead:

     * Optional: not supported by gcc

The first of its kind, normally it is clang/icc there ;-)

We could nevertheless have the link there, something like:

    * Optional: not supported by GCC
                https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223

Cheers,
Miguel
Nick Desaulniers June 21, 2021, 6:10 p.m. UTC | #3
On Sat, Jun 19, 2021 at 4:32 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Jun 19, 2021 at 1:26 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > I am not sure if it is best or not to have the GCC link in order to be
> > consistent with the rest of the links (they are for the docs only). Do
> > we know if GCC going to implement it soon?
>
> i.e. if GCC does not implement it yet we use elsewhere this kind of
> marker instead:
>
>      * Optional: not supported by gcc
>
> The first of its kind, normally it is clang/icc there ;-)

:^) GCC does have an attribute since GCC 7.1 for this.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223#c11
I'm moving Clang over to use that in
https://reviews.llvm.org/D104658
Once that lands, I'll send a v2 (without carrying any reviewed by tags).

>
> We could nevertheless have the link there, something like:
>
>     * Optional: not supported by GCC
>                 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
Fangrui Song June 21, 2021, 6:24 p.m. UTC | #4
On 2021-06-21, Nick Desaulniers wrote:
>On Sat, Jun 19, 2021 at 4:32 AM Miguel Ojeda
><miguel.ojeda.sandonis@gmail.com> wrote:
>>
>> On Sat, Jun 19, 2021 at 1:26 PM Miguel Ojeda
>> <miguel.ojeda.sandonis@gmail.com> wrote:
>> >
>> > I am not sure if it is best or not to have the GCC link in order to be
>> > consistent with the rest of the links (they are for the docs only). Do
>> > we know if GCC going to implement it soon?
>>
>> i.e. if GCC does not implement it yet we use elsewhere this kind of
>> marker instead:
>>
>>      * Optional: not supported by gcc
>>
>> The first of its kind, normally it is clang/icc there ;-)
>
>:^) GCC does have an attribute since GCC 7.1 for this.
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223#c11
>I'm moving Clang over to use that in
>https://reviews.llvm.org/D104658
>Once that lands, I'll send a v2 (without carrying any reviewed by tags).

Thanks! __attribute__((no_profile_instrument_function)) looks good to me.

Also a reminder that __GCC4_has_attribute___no_profile in v1 misses two
underscores. v2 no_profile_instrument_function may need to fix this.


Reviewed-by: Fangrui Song <maskray@google.com>

>>
>> We could nevertheless have the link there, something like:
>>
>>     * Optional: not supported by GCC
>>                 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
>
>-- 
>Thanks,
>~Nick Desaulniers
Miguel Ojeda June 21, 2021, 7:09 p.m. UTC | #5
On Mon, Jun 21, 2021 at 8:24 PM Fangrui Song <maskray@google.com> wrote:
>
> Also a reminder that __GCC4_has_attribute___no_profile in v1 misses two
> underscores. v2 no_profile_instrument_function may need to fix this.

Good catch! Yes, it is missing the last two.

Cheers,
Miguel
diff mbox series

Patch

diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index c043b8d2b17b..cf584a1908b3 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -33,6 +33,7 @@ 
 # define __GCC4_has_attribute___externally_visible__  1
 # define __GCC4_has_attribute___no_caller_saved_registers__ 0
 # define __GCC4_has_attribute___noclone__             1
+# define __GCC4_has_attribute___no_profile            0
 # define __GCC4_has_attribute___nonstring__           0
 # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
 # define __GCC4_has_attribute___no_sanitize_undefined__ (__GNUC_MINOR__ >= 9)
@@ -237,6 +238,17 @@ 
 # define __nonstring
 #endif
 
+/*
+ * Optional: only supported since clang >= 13
+ *      gcc: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
+ *    clang: https://clang.llvm.org/docs/AttributeReference.html#no_profile
+ */
+#if __has_attribute(__no_profile__)
+# define __no_profile                  __attribute__((__no_profile__))
+#else
+# define __no_profile
+#endif
+
 /*
  *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
  * clang: https://clang.llvm.org/docs/AttributeReference.html#noreturn
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index d29bda7f6ebd..d509169860f1 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -210,7 +210,7 @@  struct ftrace_likely_data {
 /* Section for code which can't be instrumented at all */
 #define noinstr								\
 	noinline notrace __attribute((__section__(".noinstr.text")))	\
-	__no_kcsan __no_sanitize_address
+	__no_kcsan __no_sanitize_address __no_profile
 
 #endif /* __KERNEL__ */