diff mbox series

[COMMITTED] bpf: define __bpf__ as well as __BPF__ as a target macro

Message ID 87edwyst58.fsf@oracle.com
State New, archived
Headers
Series [COMMITTED] bpf: define __bpf__ as well as __BPF__ as a target macro |

Commit Message

Jose E. Marchesi Aug. 29, 2022, 8:15 p.m. UTC 
  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(+)

Comments

Fangrui Song Aug. 30, 2022, 12:31 a.m. UTC | #1 
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?
Jose E. Marchesi Aug. 30, 2022, 4:45 p.m. UTC | #2 
> 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.
Fangrui Song Aug. 30, 2022, 5:24 p.m. UTC | #3 
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.
diff mbox series

Patch

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__");