public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [COMMITTED] bpf: define __bpf__ as well as __BPF__ as a target macro
@ 2022-08-29 20:15 Jose E. Marchesi
  2022-08-30  0:31 ` Fangrui Song
  0 siblings, 1 reply; 4+ messages in thread
From: Jose E. Marchesi @ 2022-08-29 20:15 UTC (permalink / raw)
  To: gcc-patches


LLVM defines both __bpf__ and __BPF_ as target macros.
GCC was defining only __BPF__.

This patch defines __bpf__ as a target macro for BPF.
Tested in bpf-unknown-none.

gcc/ChangeLog:

	* config/bpf/bpf.cc (bpf_target_macros): Define __bpf__ as a
	target macro.
---
 gcc/config/bpf/bpf.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
index 7e37e080808..9cb56cfb287 100644
--- a/gcc/config/bpf/bpf.cc
+++ b/gcc/config/bpf/bpf.cc
@@ -291,6 +291,7 @@ void
 bpf_target_macros (cpp_reader *pfile)
 {
   builtin_define ("__BPF__");
+  builtin_define ("__bpf__");
 
   if (TARGET_BIG_ENDIAN)
     builtin_define ("__BPF_BIG_ENDIAN__");
-- 
2.30.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [COMMITTED] bpf: define __bpf__ as well as __BPF__ as a target macro
  2022-08-29 20:15 [COMMITTED] bpf: define __bpf__ as well as __BPF__ as a target macro Jose E. Marchesi
@ 2022-08-30  0:31 ` Fangrui Song
  2022-08-30 16:45   ` Jose E. Marchesi
  0 siblings, 1 reply; 4+ messages in thread
From: Fangrui Song @ 2022-08-30  0:31 UTC (permalink / raw)
  To: Jose E. Marchesi; +Cc: gcc-patches

On Mon, Aug 29, 2022 at 1:16 PM Jose E. Marchesi via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> LLVM defines both __bpf__ and __BPF_ as target macros.
> GCC was defining only __BPF__.
>
> This patch defines __bpf__ as a target macro for BPF.
> Tested in bpf-unknown-none.
>
> gcc/ChangeLog:
>
>         * config/bpf/bpf.cc (bpf_target_macros): Define __bpf__ as a
>         target macro.
> ---
>  gcc/config/bpf/bpf.cc | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> index 7e37e080808..9cb56cfb287 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -291,6 +291,7 @@ void
>  bpf_target_macros (cpp_reader *pfile)
>  {
>    builtin_define ("__BPF__");
> +  builtin_define ("__bpf__");
>
>    if (TARGET_BIG_ENDIAN)
>      builtin_define ("__BPF_BIG_ENDIAN__");
> --
> 2.30.2
>

Having multiple choices in this case seems to just add confusion to
users and making code search slightly more inconvenient.

How much code uses LLVM specific __bpf__? Can it be migrated? Should
LLVM undefine the macro instead?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [COMMITTED] bpf: define __bpf__ as well as __BPF__ as a target macro
  2022-08-30  0:31 ` Fangrui Song
@ 2022-08-30 16:45   ` Jose E. Marchesi
  2022-08-30 17:24     ` Fangrui Song
  0 siblings, 1 reply; 4+ messages in thread
From: Jose E. Marchesi @ 2022-08-30 16:45 UTC (permalink / raw)
  To: Fangrui Song; +Cc: gcc-patches


> On Mon, Aug 29, 2022 at 1:16 PM Jose E. Marchesi via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>>
>> LLVM defines both __bpf__ and __BPF_ as target macros.
>> GCC was defining only __BPF__.
>>
>> This patch defines __bpf__ as a target macro for BPF.
>> Tested in bpf-unknown-none.
>>
>> gcc/ChangeLog:
>>
>>         * config/bpf/bpf.cc (bpf_target_macros): Define __bpf__ as a
>>         target macro.
>> ---
>>  gcc/config/bpf/bpf.cc | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
>> index 7e37e080808..9cb56cfb287 100644
>> --- a/gcc/config/bpf/bpf.cc
>> +++ b/gcc/config/bpf/bpf.cc
>> @@ -291,6 +291,7 @@ void
>>  bpf_target_macros (cpp_reader *pfile)
>>  {
>>    builtin_define ("__BPF__");
>> +  builtin_define ("__bpf__");
>>
>>    if (TARGET_BIG_ENDIAN)
>>      builtin_define ("__BPF_BIG_ENDIAN__");
>> --
>> 2.30.2
>>
>
> Having multiple choices in this case seems to just add confusion to
> users and making code search slightly more inconvenient.
>
> How much code uses LLVM specific __bpf__? Can it be migrated? Should
> LLVM undefine the macro instead?

I agree that it would be better to support just one form of the target
macro.  Having two alternative forms can only lead to problems.

But I think the train left the station long ago to do any better: there
are files in the kernel tree that rely on __bpf__ and there may be BPF
programs around doing the same thing.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [COMMITTED] bpf: define __bpf__ as well as __BPF__ as a target macro
  2022-08-30 16:45   ` Jose E. Marchesi
@ 2022-08-30 17:24     ` Fangrui Song
  0 siblings, 0 replies; 4+ messages in thread
From: Fangrui Song @ 2022-08-30 17:24 UTC (permalink / raw)
  To: Jose E. Marchesi; +Cc: gcc-patches

On Tue, Aug 30, 2022 at 9:46 AM Jose E. Marchesi
<jose.marchesi@oracle.com> wrote:
>
>
> > On Mon, Aug 29, 2022 at 1:16 PM Jose E. Marchesi via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >>
> >> LLVM defines both __bpf__ and __BPF_ as target macros.
> >> GCC was defining only __BPF__.
> >>
> >> This patch defines __bpf__ as a target macro for BPF.
> >> Tested in bpf-unknown-none.
> >>
> >> gcc/ChangeLog:
> >>
> >>         * config/bpf/bpf.cc (bpf_target_macros): Define __bpf__ as a
> >>         target macro.
> >> ---
> >>  gcc/config/bpf/bpf.cc | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> >> index 7e37e080808..9cb56cfb287 100644
> >> --- a/gcc/config/bpf/bpf.cc
> >> +++ b/gcc/config/bpf/bpf.cc
> >> @@ -291,6 +291,7 @@ void
> >>  bpf_target_macros (cpp_reader *pfile)
> >>  {
> >>    builtin_define ("__BPF__");
> >> +  builtin_define ("__bpf__");
> >>
> >>    if (TARGET_BIG_ENDIAN)
> >>      builtin_define ("__BPF_BIG_ENDIAN__");
> >> --
> >> 2.30.2
> >>
> >
> > Having multiple choices in this case seems to just add confusion to
> > users and making code search slightly more inconvenient.
> >
> > How much code uses LLVM specific __bpf__? Can it be migrated? Should
> > LLVM undefine the macro instead?
>
> I agree that it would be better to support just one form of the target
> macro.  Having two alternative forms can only lead to problems.
>
> But I think the train left the station long ago to do any better: there
> are files in the kernel tree that rely on __bpf__ and there may be BPF
> programs around doing the same thing.

Ok, thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-08-30 17:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-29 20:15 [COMMITTED] bpf: define __bpf__ as well as __BPF__ as a target macro Jose E. Marchesi
2022-08-30  0:31 ` Fangrui Song
2022-08-30 16:45   ` Jose E. Marchesi
2022-08-30 17:24     ` Fangrui Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).