libcpp: Use [[likely]] conditionally

Message ID 20211122172235.19438-1-polacek@redhat.com
State New
Headers show
Series
  • libcpp: Use [[likely]] conditionally
Related show

Commit Message

Petter Tomner via Gcc-patches Nov. 22, 2021, 5:22 p.m.
Let's hide [[likely]] behind a macro, to suppress warnings if the
compiler doesn't support it.

Co-authored-by: Jonathan Wakely <jwakely@redhat.com>

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

	PR preprocessor/103355

libcpp/ChangeLog:

	* lex.c: Use ATTR_LIKELY instead of [[likely]].
	* system.h (ATTR_LIKELY): Define.
---
 libcpp/lex.c    |  2 +-
 libcpp/system.h | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)


base-commit: 1aedb3920a45bfe75db4514502b4e7f83e108f63
-- 
2.33.1

Comments

Petter Tomner via Gcc-patches Nov. 23, 2021, 12:26 a.m. | #1
On 11/22/2021 10:22 AM, Marek Polacek via Gcc-patches wrote:
> Let's hide [[likely]] behind a macro, to suppress warnings if the

> compiler doesn't support it.

>

> Co-authored-by: Jonathan Wakely <jwakely@redhat.com>

>

> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

>

> 	PR preprocessor/103355

>

> libcpp/ChangeLog:

>

> 	* lex.c: Use ATTR_LIKELY instead of [[likely]].

> 	* system.h (ATTR_LIKELY): Define.

OK
jeff
Petter Tomner via Gcc-patches Nov. 23, 2021, 3:26 p.m. | #2
Hi!

On 23/11/2021 01:26, Jeff Law via Gcc-patches wrote:
>

>

> On 11/22/2021 10:22 AM, Marek Polacek via Gcc-patches wrote:

>> Let's hide [[likely]] behind a macro, to suppress warnings if the

>> compiler doesn't support it.

>>

>> Co-authored-by: Jonathan Wakely <jwakely@redhat.com>

>>

>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

>>

>>     PR preprocessor/103355

>>

>> libcpp/ChangeLog:

>>

>>     * lex.c: Use ATTR_LIKELY instead of [[likely]].

>>     * system.h (ATTR_LIKELY): Define.

> OK

> jeff



This patch breaks the build when the host compiler is gcc-4.8.5, because 
__has_cpp_attribute is not defined.

Is this small patch OK with a proper ChangeLog?


diff --git a/libcpp/system.h b/libcpp/system.h
index f6fc583ab80..b78ab813d2f 100644
--- a/libcpp/system.h
+++ b/libcpp/system.h
@@ -430,6 +430,8 @@ extern void fancy_abort (const char *, int, const 
char *) ATTRIBUTE_NORETURN;
  # else
  #  define ATTR_LIKELY
  # endif
+#else
+# define ATTR_LIKELY
  #endif

  /* Poison identifiers we do not want to use.  */


Thanks,


Christophe
Petter Tomner via Gcc-patches Nov. 23, 2021, 3:38 p.m. | #3
On Tue, Nov 23, 2021 at 04:26:19PM +0100, Christophe LYON via Gcc-patches wrote:
> Hi!

> 

> On 23/11/2021 01:26, Jeff Law via Gcc-patches wrote:

> > 

> > 

> > On 11/22/2021 10:22 AM, Marek Polacek via Gcc-patches wrote:

> > > Let's hide [[likely]] behind a macro, to suppress warnings if the

> > > compiler doesn't support it.

> > > 

> > > Co-authored-by: Jonathan Wakely <jwakely@redhat.com>

> > > 

> > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

> > > 

> > >     PR preprocessor/103355

> > > 

> > > libcpp/ChangeLog:

> > > 

> > >     * lex.c: Use ATTR_LIKELY instead of [[likely]].

> > >     * system.h (ATTR_LIKELY): Define.

> > OK

> > jeff

> 

> 

> This patch breaks the build when the host compiler is gcc-4.8.5, because

> __has_cpp_attribute is not defined.

 
Ah, of course.

> Is this small patch OK with a proper ChangeLog?


Yes, please.

> diff --git a/libcpp/system.h b/libcpp/system.h

> index f6fc583ab80..b78ab813d2f 100644

> --- a/libcpp/system.h

> +++ b/libcpp/system.h

> @@ -430,6 +430,8 @@ extern void fancy_abort (const char *, int, const char

> *) ATTRIBUTE_NORETURN;

>  # else

>  #  define ATTR_LIKELY

>  # endif

> +#else

> +# define ATTR_LIKELY

>  #endif

> 

>  /* Poison identifiers we do not want to use.  */

> 

> 

> Thanks,

> 

> 

> Christophe

> 

> 

> 


Marek
Petter Tomner via Gcc-patches Nov. 23, 2021, 3:38 p.m. | #4
On 11/23/2021 8:26 AM, Christophe LYON via Gcc-patches wrote:
> Hi!

>

> On 23/11/2021 01:26, Jeff Law via Gcc-patches wrote:

>>

>>

>> On 11/22/2021 10:22 AM, Marek Polacek via Gcc-patches wrote:

>>> Let's hide [[likely]] behind a macro, to suppress warnings if the

>>> compiler doesn't support it.

>>>

>>> Co-authored-by: Jonathan Wakely <jwakely@redhat.com>

>>>

>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

>>>

>>>     PR preprocessor/103355

>>>

>>> libcpp/ChangeLog:

>>>

>>>     * lex.c: Use ATTR_LIKELY instead of [[likely]].

>>>     * system.h (ATTR_LIKELY): Define.

>> OK

>> jeff

>

>

> This patch breaks the build when the host compiler is gcc-4.8.5, 

> because __has_cpp_attribute is not defined.

Sigh.  I'd like to move to a more recent prereq if we could.


>

> Is this small patch OK with a proper ChangeLog?

Yes.  Sorry about the breakage.
jeff
Petter Tomner via Gcc-patches Nov. 23, 2021, 8:34 p.m. | #5
On Tue, Nov 23, 2021 at 4:41 PM Jeff Law via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:

>

>

> On 11/23/2021 8:26 AM, Christophe LYON via Gcc-patches wrote:

> > Hi!

> >

> > On 23/11/2021 01:26, Jeff Law via Gcc-patches wrote:

> >>

> >>

> >> On 11/22/2021 10:22 AM, Marek Polacek via Gcc-patches wrote:

> >>> Let's hide [[likely]] behind a macro, to suppress warnings if the

> >>> compiler doesn't support it.

> >>>

> >>> Co-authored-by: Jonathan Wakely <jwakely@redhat.com>

> >>>

> >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

> >>>

> >>>     PR preprocessor/103355

> >>>

> >>> libcpp/ChangeLog:

> >>>

> >>>     * lex.c: Use ATTR_LIKELY instead of [[likely]].

> >>>     * system.h (ATTR_LIKELY): Define.

> >> OK

> >> jeff

> >

> >

> > This patch breaks the build when the host compiler is gcc-4.8.5,

> > because __has_cpp_attribute is not defined.

> Sigh.  I'd like to move to a more recent prereq if we could.

>


I don't know why we have such an old dependency indeed.
I am not requesting it, I just happen to have an old enough host
compiler so that I can check/complain when we accidentally
break the dependency :-)

Christophe



>

>

> >

> > Is this small patch OK with a proper ChangeLog?

> Yes.  Sorry about the breakage.

> jeff

>

>

>
Petter Tomner via Gcc-patches Nov. 23, 2021, 8:43 p.m. | #6
On Tue, Nov 23, 2021 at 09:34:04PM +0100, Christophe Lyon via Gcc-patches wrote:
> > > This patch breaks the build when the host compiler is gcc-4.8.5,

> > > because __has_cpp_attribute is not defined.

> > Sigh.  I'd like to move to a more recent prereq if we could.

> >

> 

> I don't know why we have such an old dependency indeed.

> I am not requesting it, I just happen to have an old enough host

> compiler so that I can check/complain when we accidentally

> break the dependency :-)


4.8.5 is still widely used and is the first one that supports C++11
reasonably well that it can be used.
__has_cpp_attribute has been added I think only in C++20, before that it was
in SD6, but even that is post C++11 I believe.
So provided we want to support C++11 (and IMHO we should, we can't afford to
be like Rust that can't build with a few days old compiler), we need to be
prepared that __has_cpp_attribute won't be defined.

	Jakub
Petter Tomner via Gcc-patches Nov. 23, 2021, 8:43 p.m. | #7
On 11/23/2021 1:34 PM, Christophe Lyon wrote:
>

>

> On Tue, Nov 23, 2021 at 4:41 PM Jeff Law via Gcc-patches 

> <gcc-patches@gcc.gnu.org> wrote:

>

>

>

>     On 11/23/2021 8:26 AM, Christophe LYON via Gcc-patches wrote:

>     > Hi!

>     >

>     > On 23/11/2021 01:26, Jeff Law via Gcc-patches wrote:

>     >>

>     >>

>     >> On 11/22/2021 10:22 AM, Marek Polacek via Gcc-patches wrote:

>     >>> Let's hide [[likely]] behind a macro, to suppress warnings if the

>     >>> compiler doesn't support it.

>     >>>

>     >>> Co-authored-by: Jonathan Wakely <jwakely@redhat.com>

>     >>>

>     >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

>     >>>

>     >>>     PR preprocessor/103355

>     >>>

>     >>> libcpp/ChangeLog:

>     >>>

>     >>>     * lex.c: Use ATTR_LIKELY instead of [[likely]].

>     >>>     * system.h (ATTR_LIKELY): Define.

>     >> OK

>     >> jeff

>     >

>     >

>     > This patch breaks the build when the host compiler is gcc-4.8.5,

>     > because __has_cpp_attribute is not defined.

>     Sigh.  I'd like to move to a more recent prereq if we could.

>

>

> I don't know why we have such an old dependency indeed.

> I am not requesting it, I just happen to have an old enough host

> compiler so that I can check/complain when we accidentally

> break the dependency :-)

Probably the enterprise distros.  I suspect we'll be able to roll 
forward in 2-3 years...

Jeff

Patch

diff --git a/libcpp/lex.c b/libcpp/lex.c
index 94c36f0d014..9c27d8b5a08 100644
--- a/libcpp/lex.c
+++ b/libcpp/lex.c
@@ -1286,7 +1286,7 @@  namespace bidi {
       case kind::RTL:
 	/* These aren't popped by a PDF/PDI.  */
 	break;
-      [[likely]] case kind::NONE:
+      ATTR_LIKELY case kind::NONE:
 	break;
       default:
 	abort ();
diff --git a/libcpp/system.h b/libcpp/system.h
index ee5fbe28889..f6fc583ab80 100644
--- a/libcpp/system.h
+++ b/libcpp/system.h
@@ -422,6 +422,16 @@  extern void fancy_abort (const char *, int, const char *) ATTRIBUTE_NORETURN;
 #define gcc_checking_assert(EXPR) ((void)(0 && (EXPR)))
 #endif
 
+#ifdef __has_cpp_attribute
+# if __has_cpp_attribute(likely)
+#  define ATTR_LIKELY [[likely]]
+# elif __has_cpp_attribute(__likely__)
+#  define ATTR_LIKELY [[__likely__]]
+# else
+#  define ATTR_LIKELY
+# endif
+#endif
+
 /* Poison identifiers we do not want to use.  */
 #if (GCC_VERSION >= 3000)
 #undef calloc