Hi Koakuma, On Sat, Jun 22, 2024 at 12:18:17PM +0000, Koakuma wrote: > Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > > > I saw through the LLVM issue above that one other patch is necessary to > > fix an issue in the vDSO [1], which I applied in testing this one. > > Mhmm, I did not submit that yet because I don't feel fully confident > with it. I think it should probably live in include/vdso/math64.h > as plain C code instead of the current asm version, but I don't know > what is the proper way to check the current environment's word size. > Is checking BITS_PER_LONG enough, or should I do it in another way? Yes, I believe that is what BITS_PER_LONG is there for, you will see other checks in the tree for that. You could also reach out to the maintainers of the generic vDSO infrastructure to see if they have any ideas or suggestions for integration. > > I noticed in applying that change that you appear to be working on 6.1, > > which is fine for now, but you'll need another diff once you get to a > > newer version, as we stopped using CROSS_COMPILE to set clang's > > '--target=' value: > > > > diff --git a/scripts/Makefile.clang b/scripts/Makefile.clang > > index 6c23c6af797f..2435efae67f5 100644 > > --- a/scripts/Makefile.clang > > +++ b/scripts/Makefile.clang > > @@ -10,6 +10,7 @@ CLANG_TARGET_FLAGS_mips := mipsel-linux-gnu > > CLANG_TARGET_FLAGS_powerpc := powerpc64le-linux-gnu > > CLANG_TARGET_FLAGS_riscv := riscv64-linux-gnu > > CLANG_TARGET_FLAGS_s390 := s390x-linux-gnu > > +CLANG_TARGET_FLAGS_sparc := sparc64-linux-gnu > > CLANG_TARGET_FLAGS_x86 := x86_64-linux-gnu > > CLANG_TARGET_FLAGS_um := $(CLANG_TARGET_FLAGS_$(SUBARCH)) > > CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(SRCARCH)) > > Yeah, I was working with 6.1 at that time since it's the version > that my distro have installed for me. Now this is more of a workflow That makes sense. I do think you should start working off of a more recent version (ideally at least mainline) for your future revisions, just so that your patches can be applied with less friction on maintainers. That can help your patches get picked up quicker :) > question, but this means I should submit a v2 with this change > merged in with mine too, right? Here is what I would do: 1. Either keep this patch the way that it is or break it up into two separate patches (especially given Adrian's other review comment): One for removing the '-fcall-used' flags, with the comments about how it does not impact the ABI and the registers can still be used in assembly if needed, perhaps with some benchmarks with any codegen? Might not be strictly necessary since Sam did not seem opposed in the previous discussion. One for changing the vDSO from '-mv8plus' to '-mcpu=v9' (if this is still okay). 2. Add another patch with that diff above with some notes about what was tested to justify allowing this now. You'll end up with either a two or three patch series. I would send this series to both the SPARC people that you have added here along with the Kbuild and ClangBuiltLinux folks, which you can get from the output of $ scripts/get_maintainers.pl scripts/Makefile.clang or use 'b4 prep --auto-to-cc' after crafting the series, since it appears you used it for this series. For the cover letter, you can add some commentary about what was tested and request integration from either the SPARC folks or Masahiro, depending on who wants to carry the changes, since they should go through one tree atomically ideally. If you have any questions about or issues with that comment or any other aspect of this process, I am happy to answer or clarify as necessary! I am in the #clang-built-linux channel in the LLVM Discord and #clangbuiltlinux on Libera if anything comes up. > And thanks for the feedback! Always happy to help get more people involved with the kernel, especially from the clang/LLVM side :) Cheers, Nathan
