Re: [PATCH] libcpp: Use [[likely]] conditionally

2021-11-23 Thread Jeff Law via Gcc-patches




On 11/23/2021 1:34 PM, Christophe Lyon wrote:



On Tue, Nov 23, 2021 at 4:41 PM Jeff Law via Gcc-patches 
 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 
>>>
>>> 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


Re: [PATCH] libcpp: Use [[likely]] conditionally

2021-11-23 Thread Jakub Jelinek via Gcc-patches
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



Re: [PATCH] libcpp: Use [[likely]] conditionally

2021-11-23 Thread Christophe Lyon via Gcc-patches
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 
> >>>
> >>> 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
>
>
>


Re: [PATCH] libcpp: Use [[likely]] conditionally

2021-11-23 Thread Jeff Law via Gcc-patches




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 

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




Re: [PATCH] libcpp: Use [[likely]] conditionally

2021-11-23 Thread Marek Polacek via Gcc-patches
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 
> > > 
> > > 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



Re: [PATCH] libcpp: Use [[likely]] conditionally

2021-11-23 Thread Christophe LYON via Gcc-patches

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 

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





Re: [PATCH] libcpp: Use [[likely]] conditionally

2021-11-22 Thread Jeff Law via Gcc-patches




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 

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



[PATCH] libcpp: Use [[likely]] conditionally

2021-11-22 Thread Marek Polacek via Gcc-patches
Let's hide [[likely]] behind a macro, to suppress warnings if the
compiler doesn't support it.

Co-authored-by: Jonathan Wakely 

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(-)

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

base-commit: 1aedb3920a45bfe75db4514502b4e7f83e108f63
-- 
2.33.1