On Tue, Feb 11, 2025 at 08:51:07PM +0000, Peilin Ye wrote:
> > > > Nit: why is dummy_test() necessary?
> > >
> > > It's just to make it clear when these tests are (effectively) skipped.
<...>
> > > Commit 147c8f4470ee ("selftests/bpf: Add unit tests for new
> > > sign-extension load insns") did similar thing in verifier_ldsx.c.
> >
> > I see, thank you for explaining.
> > We do have a concept of skipped tests in the test-suite,
> > but it is implemented by calling test__skip() from the prog_tests/<smth>.c.
> > This would translate as something like below in prog_tests/verifier.c:
> >
> > void test_verifier_store_release(void) {
> > #if defined(ENABLE_ATOMICS_TESTS) && defined(__TARGET_ARCH_arm64)
> > RUN(verifier_store_release);
> > #else
> > test__skip()
> > #endif
> > }
>
> > The number of tests skipped is printed after tests execution.
I tried:
void test_verifier_load_acquire(void)
{
#if __clang_major__ >= 18 && defined(ENABLE_ATOMICS_TESTS) && defined(__aarch64__)
RUN(verifier_load_acquire);
#else
printf("%s:SKIP: Clang version < 18, ENABLE_ATOMICS_TESTS not defined, and/or JIT doesn't support load-acquire\n",
__func__);
test__skip();
#endif
}
Then realized that I can't check __clang_major__ in .../prog_tests/*
files (e.g. I was building prog_tests/verifier.c using GCC). I think
ideally we should do something similar to prog{,_test}s/arena_atomics.c,
i.e. use a global bool in BPF source to indicate if we should skip this
test, but that seems to require non-trivial changes to
prog_tests/verifier.c?
For the purpose of this patchset, let me keep dummy_test(), like what we
have now in verifier_ldsx.c.
Thanks,
Peilin Ye
