Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpf: Replace deprecated "-target bpf" with "--target=bpf" for clang #26553

Merged
merged 1 commit into from Jun 30, 2023

Conversation

qmonnet
Copy link
Member

@qmonnet qmonnet commented Jun 29, 2023

Passing the target to clang with -target <name> has been considered deprecated since clang 3.4, released in 2013, in favour of the more recent syntax --target=<name>. Let's update our code and documentation to use the recommended syntax.

This replicates a similar patch submitted to the Linux kernel.

Passing the target to clang with "-target <name>" has been considered
deprecated since clang 3.4 [0], released in 2013, in favour of the more
recent syntax "--target=<name>". Let's update our code and documentation
to use the recommended syntax.

This replicates a similar patch submitted to the Linux kernel [1].

[0] llvm/llvm-project@274b6f0
[1] https://lore.kernel.org/all/20230624001856.1903733-1-maskray@google.com/
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@qmonnet qmonnet added sig/loader Impacts the loading of BPF programs into the kernel. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. labels Jun 29, 2023
@qmonnet qmonnet requested review from a team as code owners June 29, 2023 08:35
@qmonnet
Copy link
Member Author

qmonnet commented Jun 29, 2023

/test

@lmb
Copy link
Contributor

lmb commented Jun 29, 2023

FWIW, clang command line args seem like a mess. They mix double dashes, single dashes, space separated, equals separated.

Another point to think about: what kind of flags does gcc accept? Might make it easier to experiment with bpf-gcc in the future if both compilers agree.

@qmonnet
Copy link
Member Author

qmonnet commented Jun 29, 2023

FWIW, clang command line args seem like a mess. They mix double dashes, single dashes, space separated, equals separated.

@lmb Yep, I noticed :). I don't mind much between the two syntaxes to be honest, I simply thought it would be best to remain in sync with whatever the kernel goes with.

Another point to think about: what kind of flags does gcc accept? Might make it easier to experiment with bpf-gcc in the future if both compilers agree.

That's a good question. So apparently there's no direct equivalent to --target=bpf for GCC, instead you compile your BPF-dedicated version of GCC and call that. See for example the flags used on godbolt.org:

image

To compare with those used with clang:

image

(There is a --target=bpf flag mentioned on the reference page for the BPF backend, but it's used for compiling the BPF version of GCC, not to pick BPF with a generic GCC).

GCC does have a number of eBPF-related options though, none being identical to clang's, so the command line invocations won't be interchangeable anyway.

image

Thanks for the reviews!

Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@borkmann borkmann added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jun 30, 2023
@borkmann borkmann merged commit dff7aa1 into cilium:main Jun 30, 2023
66 checks passed
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 30, 2023
@qmonnet qmonnet deleted the pr/target-bpf branch June 30, 2023 08:43
@joamaki joamaki mentioned this pull request Jul 5, 2023
23 tasks
@joamaki joamaki added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 5, 2023
@jibi jibi added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. sig/loader Impacts the loading of BPF programs into the kernel.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants