BPF List
 help / color / mirror / Atom feed
From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Cupertino Miranda <cupertino.miranda@oracle.com>,
	bpf@vger.kernel.org, Yonghong Song <yonghong.song@linux.dev>,
	Eduard Zingerman <eddyz87@gmail.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	david.faust@oracle.com
Subject: Re: [PATCH] libbpf: add support to GCC in CORE macro definitions
Date: Thu, 08 Feb 2024 20:54:57 +0100	[thread overview]
Message-ID: <87wmreafzi.fsf@oracle.com> (raw)
In-Reply-To: <CAEf4BzZ5=E+EFs4vccWr-NPpqHej915w-GQfhSG=F1RaAJXB-A@mail.gmail.com> (Andrii Nakryiko's message of "Thu, 8 Feb 2024 10:50:16 -0800")


> On Thu, Feb 8, 2024 at 5:07 AM Cupertino Miranda
> <cupertino.miranda@oracle.com> wrote:
>>
>>
>> Hi everyone,
>>
>> This is a patch to make CORE builtin macros work with builtin
>> implementation within GCC.
>>
>> Looking forward to your comments.
>>
>
> Can you please repost it as a proper patch email, not as an attachment?
>
> But generally speaking, is there any way to change/fix GCC to allow a
> much more straightforward way to capture type, similar to how Clang
> does it? I'm not a big fan of extern declarations and using per-file
> __COUNTER__. Externs are globally visible and we can potentially run
> into name conflicts because __COUNTER__ is not globally unique.
>
> And just in general, it seems like this shouldn't require such
> acrobatics.
>
> Jose, do you have any thoughts on this?

Yes the macro is ugly and more elaborated than the clang version, but I
am afraid it is necessary in order to overcome the fact GCC
constant-folds enumerated values at parse-time.

Note however that the expression-statement itself to which the macro
expands is not elaborated, much like the null pointer dereference in the
clang version doesn't get elaborated.  These are just conveyed to the
builtins an the builtins use the TREE (IR in case of clang I guess) to
extract the type from it.

As far as I understand it the extern declaration in the macro is not
declaring an object with extern visibility, so it should not result in
any symbol being defined nor have any impact outside of the compilation
unit.  The __COUNTER__ is there just so you can use the macro more than
once in the same compilation unit, but that's all.

Cuper will correct me if I am wrong.

>
>> Regards,
>> Cupertino
>>

  reply	other threads:[~2024-02-08 19:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-08 13:04 [PATCH] libbpf: add support to GCC in CORE macro definitions Cupertino Miranda
2024-02-08 18:50 ` Andrii Nakryiko
2024-02-08 19:54   ` Jose E. Marchesi [this message]
2024-02-08 20:11     ` Cupertino Miranda
  -- strict thread matches above, loose matches on Subject: below --
2024-02-08 19:58 Cupertino Miranda
2024-02-12 23:51 ` Andrii Nakryiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wmreafzi.fsf@oracle.com \
    --to=jose.marchesi@oracle.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=cupertino.miranda@oracle.com \
    --cc=david.faust@oracle.com \
    --cc=eddyz87@gmail.com \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox