On Fri, 1 Dec 2023 10:19:01 GMT, Andrew Haley <a...@openjdk.org> wrote:
>> Xiaohong Gong has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains ten additional >> commits since the last revision: >> >> - Separate neon and sve functions into two source files >> - Merge branch 'jdk:master' into JDK-8312425 >> - Rename vmath to sleef in configure >> - Address review comments in build system >> - Add a bundled native lib in jdk as a bridge to libsleef >> - Merge 'jdk:master' into JDK-8312425 >> - Disable sleef by default >> - Merge 'jdk:master' into JDK-8312425 >> - 8312425: [vectorapi] AArch64: Optimize vector math operations with SLEEF > > Oh, and: > > If we can't trust SLEEF not to break the ABI we're using, we should not be > using SLEEF. @theRealAph You are making good points. You are basically saying: "we don't need any configure support for libsleef, we can just hard-code the names and dispatch to them directly to a dlopened library at runtime". That is technically correct, but I'd still like to argue that the current setup have some merits: * It will check that there is no typo in the function names. I agree that the likelihood of getting this wrong is low, but it is still a good practice to use official include files to have the compiler help checking this. * If we would like to bundle libsleef.so with the JVM, now we have the possibility do do so easily. (Especially if it is like you say that it is not commonly installed). (If licenses allow etc) * If we want to incorporate/bundle the source code of libsleef into OpenJDK, and build it as part of our internal library, we will have a good starting position, compared to starting from a hard-coded assembly file in hotspot. (I thought I heard some noise about this prospect). So at this point, I am okay with the general approach of this PR. There are still some build issues to sort out, though, I'll address them separately. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16234#issuecomment-1836266314
