diff mbox series

predict: Don't emit -Wsuggest-attribute=cold warning for functions which already have that attribute [PR105685]

Message ID ZB7EJsY28ImtWfLF@tucnak
State New
Headers
Series predict: Don't emit -Wsuggest-attribute=cold warning for functions which already have that attribute [PR105685] |

Commit Message

Jakub Jelinek March 25, 2023, 9:53 a.m. UTC 
  Hi!

In the following testcase, we predict baz to have cold
entry regardless of the user supplied attribute (as it call
unconditionally a cold function), but still issue
a -Wsuggest-attribute=cold warning despite it having that attribute
already.

The following patch avoids that.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-03-25  Jakub Jelinek  <jakub@redhat.com>

	PR ipa/105685
	* predict.cc (compute_function_frequency): Don't call
	warn_function_cold if function already has cold attribute.

	* c-c++-common/cold-2.c: New test.


	Jakub

Comments

Jeff Law March 26, 2023, 5:57 p.m. UTC | #1 
On 3/25/23 03:53, Jakub Jelinek via Gcc-patches wrote:
> Hi!
> 
> In the following testcase, we predict baz to have cold
> entry regardless of the user supplied attribute (as it call
> unconditionally a cold function), but still issue
> a -Wsuggest-attribute=cold warning despite it having that attribute
> already.
> 
> The following patch avoids that.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2023-03-25  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR ipa/105685
> 	* predict.cc (compute_function_frequency): Don't call
> 	warn_function_cold if function already has cold attribute.
> 
> 	* c-c++-common/cold-2.c: New test.
OK
jeff
Jan Hubicka March 26, 2023, 8:01 p.m. UTC | #2 
> Hi!
> 
> In the following testcase, we predict baz to have cold
> entry regardless of the user supplied attribute (as it call
> unconditionally a cold function), but still issue
> a -Wsuggest-attribute=cold warning despite it having that attribute
> already.
> 
> The following patch avoids that.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2023-03-25  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR ipa/105685
> 	* predict.cc (compute_function_frequency): Don't call
> 	warn_function_cold if function already has cold attribute.
> 
> 	* c-c++-common/cold-2.c: New test.
> 
> --- gcc/predict.cc.jj	2023-01-02 09:32:38.273055726 +0100
> +++ gcc/predict.cc	2023-03-24 16:54:13.658606215 +0100
> @@ -4033,7 +4033,9 @@ compute_function_frequency (void)
>      }
>  
>    node->frequency = NODE_FREQUENCY_UNLIKELY_EXECUTED;
> -  warn_function_cold (current_function_decl);
> +  if (lookup_attribute ("cold", DECL_ATTRIBUTES (current_function_decl))
> +      == NULL)
> +    warn_function_cold (current_function_decl);
OK, tanks!
In general we probably want to walk aliases and suggest warning on
aliases attached to the function, but we get this wrong with other
attributes too, so I will add it to TODo for next stage1.

Honza
>    if (ENTRY_BLOCK_PTR_FOR_FN (cfun)->count.ipa() == profile_count::zero ())
>      return;
>    FOR_EACH_BB_FN (bb, cfun)
> --- gcc/testsuite/c-c++-common/cold-2.c.jj	2023-03-24 16:56:07.344000973 +0100
> +++ gcc/testsuite/c-c++-common/cold-2.c	2023-03-24 16:55:58.985119001 +0100
> @@ -0,0 +1,19 @@
> +/* PR ipa/105685 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wsuggest-attribute=cold" } */
> +
> +extern void foo (char *, char const *, int);
> +
> +__attribute__((cold)) char *
> +bar (int x)
> +{
> +  static char b[42];
> +  foo (b, "foo", x);
> +  return b;
> +}
> +
> +__attribute__((cold)) char *
> +baz (int x)		/* { dg-bogus "function might be candidate for attribute 'cold'" } */
> +{
> +  return bar (x);
> +}
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/predict.cc.jj	2023-01-02 09:32:38.273055726 +0100
+++ gcc/predict.cc	2023-03-24 16:54:13.658606215 +0100
@@ -4033,7 +4033,9 @@  compute_function_frequency (void)
     }
 
   node->frequency = NODE_FREQUENCY_UNLIKELY_EXECUTED;
-  warn_function_cold (current_function_decl);
+  if (lookup_attribute ("cold", DECL_ATTRIBUTES (current_function_decl))
+      == NULL)
+    warn_function_cold (current_function_decl);
   if (ENTRY_BLOCK_PTR_FOR_FN (cfun)->count.ipa() == profile_count::zero ())
     return;
   FOR_EACH_BB_FN (bb, cfun)
--- gcc/testsuite/c-c++-common/cold-2.c.jj	2023-03-24 16:56:07.344000973 +0100
+++ gcc/testsuite/c-c++-common/cold-2.c	2023-03-24 16:55:58.985119001 +0100
@@ -0,0 +1,19 @@ 
+/* PR ipa/105685 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wsuggest-attribute=cold" } */
+
+extern void foo (char *, char const *, int);
+
+__attribute__((cold)) char *
+bar (int x)
+{
+  static char b[42];
+  foo (b, "foo", x);
+  return b;
+}
+
+__attribute__((cold)) char *
+baz (int x)		/* { dg-bogus "function might be candidate for attribute 'cold'" } */
+{
+  return bar (x);
+}