Re: [PATCH 1/3] selftests/bpf: Update LLVM Phabricator links
Hi Alexei,
On Thu, Jan 11, 2024 at 12:00:50PM -0800, Alexei Starovoitov wrote:
> On Thu, Jan 11, 2024 at 11:40 AM Nathan Chancellor wrote:
> >
> > Hi Yonghong,
> >
> > On Wed, Jan 10, 2024 at 08:05:36PM -0800, Yonghong Song wrote:
> > >
> > > On 1/9/24 2:16 PM, Nathan Chancellor wrote:
> > > > reviews.llvm.org was LLVM's Phabricator instances for code review. It
> > > > has been abandoned in favor of GitHub pull requests. While the majority
> > > > of links in the kernel sources still work because of the work Fangrui
> > > > has done turning the dynamic Phabricator instance into a static archive,
> > > > there are some issues with that work, so preemptively convert all the
> > > > links in the kernel sources to point to the commit on GitHub.
> > > >
> > > > Most of the commits have the corresponding differential review link in
> > > > the commit message itself so there should not be any loss of fidelity in
> > > > the relevant information.
> > > >
> > > > Additionally, fix a typo in the xdpwall.c print ("LLMV" -> "LLVM") while
> > > > in the area.
> > > >
> > > > Link:
> > > > https://discourse.llvm.org/t/update-on-github-pull-requests/71540/172
> > > > Signed-off-by: Nathan Chancellor
> > >
> > > Ack with one nit below.
> > >
> > > Acked-by: Yonghong Song
> >
> >
> >
> > > > @@ -304,6 +304,6 @@ from running test_progs will look like:
> > > > .. code-block:: console
> > > > - test_xdpwall:FAIL:Does LLVM have https://reviews.llvm.org/D109073?
> > > > unexpected error: -4007
> > > > + test_xdpwall:FAIL:Does LLVM have
> > > > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d5?
> > > > unexpected error: -4007
> > > > -__ https://reviews.llvm.org/D109073
> > > > +__
> > > > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d
> > >
> > > To be consistent with other links, could you add the missing last alnum
> > > '5' to the above link?
> >
> > Thanks a lot for catching this and providing an ack. Andrew, could you
> > squash this update into selftests-bpf-update-llvm-phabricator-links.patch?
>
> Please send a new patch.
> We'd like to take all bpf patches through the bpf tree to avoid conflicts.
Very well, I've sent a standalone v2 on top of bpf-next:
https://lore.kernel.org/20240111-bpf-update-llvm-phabricator-links-v2-1-9a7ae976b...@kernel.org/
Andrew, just drop selftests-bpf-update-llvm-phabricator-links.patch
altogether in that case, the other two patches are fine to go via -mm I
think.
Cheers,
Nathan
Re: [PATCH 1/3] selftests/bpf: Update LLVM Phabricator links
On Thu, Jan 11, 2024 at 11:40 AM Nathan Chancellor wrote:
>
> Hi Yonghong,
>
> On Wed, Jan 10, 2024 at 08:05:36PM -0800, Yonghong Song wrote:
> >
> > On 1/9/24 2:16 PM, Nathan Chancellor wrote:
> > > reviews.llvm.org was LLVM's Phabricator instances for code review. It
> > > has been abandoned in favor of GitHub pull requests. While the majority
> > > of links in the kernel sources still work because of the work Fangrui
> > > has done turning the dynamic Phabricator instance into a static archive,
> > > there are some issues with that work, so preemptively convert all the
> > > links in the kernel sources to point to the commit on GitHub.
> > >
> > > Most of the commits have the corresponding differential review link in
> > > the commit message itself so there should not be any loss of fidelity in
> > > the relevant information.
> > >
> > > Additionally, fix a typo in the xdpwall.c print ("LLMV" -> "LLVM") while
> > > in the area.
> > >
> > > Link:
> > > https://discourse.llvm.org/t/update-on-github-pull-requests/71540/172
> > > Signed-off-by: Nathan Chancellor
> >
> > Ack with one nit below.
> >
> > Acked-by: Yonghong Song
>
>
>
> > > @@ -304,6 +304,6 @@ from running test_progs will look like:
> > > .. code-block:: console
> > > - test_xdpwall:FAIL:Does LLVM have https://reviews.llvm.org/D109073?
> > > unexpected error: -4007
> > > + test_xdpwall:FAIL:Does LLVM have
> > > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d5?
> > > unexpected error: -4007
> > > -__ https://reviews.llvm.org/D109073
> > > +__
> > > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d
> >
> > To be consistent with other links, could you add the missing last alnum '5'
> > to the above link?
>
> Thanks a lot for catching this and providing an ack. Andrew, could you
> squash this update into selftests-bpf-update-llvm-phabricator-links.patch?
Please send a new patch.
We'd like to take all bpf patches through the bpf tree to avoid conflicts.
Re: [PATCH 1/3] selftests/bpf: Update LLVM Phabricator links
Hi Yonghong,
On Wed, Jan 10, 2024 at 08:05:36PM -0800, Yonghong Song wrote:
>
> On 1/9/24 2:16 PM, Nathan Chancellor wrote:
> > reviews.llvm.org was LLVM's Phabricator instances for code review. It
> > has been abandoned in favor of GitHub pull requests. While the majority
> > of links in the kernel sources still work because of the work Fangrui
> > has done turning the dynamic Phabricator instance into a static archive,
> > there are some issues with that work, so preemptively convert all the
> > links in the kernel sources to point to the commit on GitHub.
> >
> > Most of the commits have the corresponding differential review link in
> > the commit message itself so there should not be any loss of fidelity in
> > the relevant information.
> >
> > Additionally, fix a typo in the xdpwall.c print ("LLMV" -> "LLVM") while
> > in the area.
> >
> > Link: https://discourse.llvm.org/t/update-on-github-pull-requests/71540/172
> > Signed-off-by: Nathan Chancellor
>
> Ack with one nit below.
>
> Acked-by: Yonghong Song
> > @@ -304,6 +304,6 @@ from running test_progs will look like:
> > .. code-block:: console
> > - test_xdpwall:FAIL:Does LLVM have https://reviews.llvm.org/D109073?
> > unexpected error: -4007
> > + test_xdpwall:FAIL:Does LLVM have
> > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d5?
> > unexpected error: -4007
> > -__ https://reviews.llvm.org/D109073
> > +__
> > https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d
>
> To be consistent with other links, could you add the missing last alnum '5'
> to the above link?
Thanks a lot for catching this and providing an ack. Andrew, could you
squash this update into selftests-bpf-update-llvm-phabricator-links.patch?
diff --git a/tools/testing/selftests/bpf/README.rst
b/tools/testing/selftests/bpf/README.rst
index b9a493f66557..e56034abb3c2 100644
--- a/tools/testing/selftests/bpf/README.rst
+++ b/tools/testing/selftests/bpf/README.rst
@@ -306,4 +306,4 @@ from running test_progs will look like:
test_xdpwall:FAIL:Does LLVM have
https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d5?
unexpected error: -4007
-__
https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d
+__
https://github.com/llvm/llvm-project/commit/ea72b0319d7b0f0c2fcf41d121afa5d031b319d5
Re: [PATCH 1/3] selftests/bpf: Update LLVM Phabricator links
On 1/9/24 2:16 PM, Nathan Chancellor wrote:
reviews.llvm.org was LLVM's Phabricator instances for code review. It
has been abandoned in favor of GitHub pull requests. While the majority
of links in the kernel sources still work because of the work Fangrui
has done turning the dynamic Phabricator instance into a static archive,
there are some issues with that work, so preemptively convert all the
links in the kernel sources to point to the commit on GitHub.
Most of the commits have the corresponding differential review link in
the commit message itself so there should not be any loss of fidelity in
the relevant information.
Additionally, fix a typo in the xdpwall.c print ("LLMV" -> "LLVM") while
in the area.
Link: https://discourse.llvm.org/t/update-on-github-pull-requests/71540/172
Signed-off-by: Nathan Chancellor
Ack with one nit below.
Acked-by: Yonghong Song
---
Cc: a...@kernel.org
Cc: dan...@iogearbox.net
Cc: and...@kernel.org
Cc: myko...@fb.com
Cc: b...@vger.kernel.org
Cc: linux-kselft...@vger.kernel.org
---
tools/testing/selftests/bpf/README.rst | 32 +++---
tools/testing/selftests/bpf/prog_tests/xdpwall.c | 2 +-
.../selftests/bpf/progs/test_core_reloc_type_id.c | 2 +-
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/bpf/README.rst
b/tools/testing/selftests/bpf/README.rst
index cb9b95702ac6..b9a493f66557 100644
--- a/tools/testing/selftests/bpf/README.rst
+++ b/tools/testing/selftests/bpf/README.rst
@@ -115,7 +115,7 @@ the insn 20 undoes map_value addition. It is currently
impossible for the
verifier to understand such speculative pointer arithmetic.
Hence `this patch`__ addresses it on the compiler side. It was committed on
llvm 12.
-__ https://reviews.llvm.org/D85570
+__
https://github.com/llvm/llvm-project/commit/ddf1864ace484035e3cde5e83b3a31ac81e059c6
The corresponding C code
@@ -165,7 +165,7 @@ This is due to a llvm BPF backend bug. `The fix`__
has been pushed to llvm 10.x release branch and will be
available in 10.0.1. The patch is available in llvm 11.0.0 trunk.
-__ https://reviews.llvm.org/D78466
+__
https://github.com/llvm/llvm-project/commit/3cb7e7bf959dcd3b8080986c62e10a75c7af43f0
bpf_verif_scale/loop6.bpf.o test failure with Clang 12
==
@@ -204,7 +204,7 @@ r5(w5) is eventually saved on stack at insn #24 for later
use.
This cause later verifier failure. The bug has been `fixed`__ in
Clang 13.
-__ https://reviews.llvm.org/D97479
+__
https://github.com/llvm/llvm-project/commit/1959ead525b8830cc8a345f45e1c3ef9902d3229
BPF CO-RE-based tests and Clang version
===
@@ -221,11 +221,11 @@ failures:
- __builtin_btf_type_id() [0_, 1_, 2_];
- __builtin_preserve_type_info(), __builtin_preserve_enum_value() [3_, 4_].
-.. _0: https://reviews.llvm.org/D74572
-.. _1: https://reviews.llvm.org/D74668
-.. _2: https://reviews.llvm.org/D85174
-.. _3: https://reviews.llvm.org/D83878
-.. _4: https://reviews.llvm.org/D83242
+.. _0:
https://github.com/llvm/llvm-project/commit/6b01b465388b204d543da3cf49efd6080db094a9
+.. _1:
https://github.com/llvm/llvm-project/commit/072cde03aaa13a2c57acf62d79876bf79aa1919f
+.. _2:
https://github.com/llvm/llvm-project/commit/00602ee7ef0bf6c68d690a2bd729c12b95c95c99
+.. _3:
https://github.com/llvm/llvm-project/commit/6d218b4adb093ff2e9764febbbc89f429412006c
+.. _4:
https://github.com/llvm/llvm-project/commit/6d6750696400e7ce988d66a1a00e1d0cb32815f8
Floating-point tests and Clang version
==
@@ -234,7 +234,7 @@ Certain selftests, e.g. core_reloc, require support for the
floating-point
types, which was introduced in `Clang 13`__. The older Clang versions will
either crash when compiling these tests, or generate an incorrect BTF.
-__ https://reviews.llvm.org/D83289
+__
https://github.com/llvm/llvm-project/commit/a7137b238a07d9399d3ae96c0b461571bd5aa8b2
Kernel function call test and Clang version
===
@@ -248,7 +248,7 @@ Without it, the error from compiling bpf selftests looks
like:
libbpf: failed to find BTF for extern 'tcp_slow_start' [25] section: -2
-__ https://reviews.llvm.org/D93563
+__
https://github.com/llvm/llvm-project/commit/886f9ff53155075bd5f1e994f17b85d1e1b7470c
btf_tag test and Clang version
==
@@ -264,8 +264,8 @@ Without them, the btf_tag selftest will be skipped and you
will observe:
# btf_tag:SKIP
-.. _0: https://reviews.llvm.org/D111588
-.. _1: https://reviews.llvm.org/D99
+.. _0:
https://github.com/llvm/llvm-project/commit/a162b67c98066218d0d00aa13b99afb95d9bb5e6
+.. _1:
https://github.com/llvm/llvm-project/commit/3466e00716e12e32fdb100e3fcfca5c2b3e8d784
Clang dependencies for static linking tests
===
@
