Re: [PATCH bpf-next v2 11/12] selftests/bpf: Add inline-asm tests for by-value arguments

[Yonghong Song <yonghong.song@linux.dev>]

On 9/9/26 12:30 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c b/tools/testing/selftests/bpf/prog_tests/aggregate_arg.c
> [ ... ]
>
>> diff --git a/tools/testing/selftests/bpf/progs/aggregate_arg_func.c b/tools/testing/selftests/bpf/progs/aggregate_arg_func.c
>> new file mode 100644
>> index 000000000000..61dd5f86c3a4
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/aggregate_arg_func.c
>> @@ -0,0 +1,155 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
>> +#include <vmlinux.h>
>> +#include <bpf/bpf_helpers.h>
>> +#include "bpf_misc.h"
>> +
>> +typedef unsigned __int128 u128;
>> +
>> +struct pair {
>> +	__u64 lo;
>> +	__u64 hi;
>> +};
>> +
>> +struct too_big {
>> +	__u64 a;
>> +	__u64 b;
>> +	__u64 c;
>> +};
>> +
>> +#if defined(__clang__)
>> +
>> +__noinline __u64 global_arg_pair(int a, struct pair p, int c)
>> +{
>> +	return (__u64)a + p.lo + p.hi + c;
>> +}
> [ ... ]
>
>> +#endif
>> +
>> +__noinline __u64 global_arg_too_big(struct too_big s)
>> +{
>> +	return s.a + s.b + s.c;
>> +}
> [ ... ]
>
>> +#if defined(__BPF_FEATURE_STACK_ARGUMENT)
>> +
>> +__noinline __u64 global_arg_split(int a, int b, int c, int d, struct pair p)
>> +{
>> +	return (__u64)a + b + c + d + p.lo + p.hi;
>> +}
> [ ... ]
>
>> +#endif
> This isn't a bug, but would it help a later reader to carry the
> changelog's gcc-invisible-reference rationale into a short comment above
> the #if defined(__clang__), and to note why the last three cases need
> __BPF_FEATURE_STACK_ARGUMENT (and maybe label the two #endif closures)?

stack argument is implemented in llvm23, and that is why we want this
feature gating.

>
> The sibling file aggregate_ret_func.c documents its equivalent gate:
>
>    /*
>     * gcc returns a by-value struct through a hidden pointer, and emits the
>     * 'r0 = r1' returning it after the __naked body's exit, leaving the
>     * subprogram falling through. Build these with clang only.
>     */
>    #if defined(__clang__)
>
> and other progs files label the closing directive, for example
> verifier_ldsx.c uses #endif /* __BPF_FEATURE_ADDR_SPACE_CAST */.

This is fine. We do not want to have this comment in many places.
The commit message should explain this.

>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34320399441