RFR: 8329816: Add SLEEF version 3.6.1 [v2]

Magnus Ihse Bursie ihse at openjdk.org
Thu Jun 27 22:00:52 UTC 2024


On Thu, 23 May 2024 12:49:42 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Okay, suffix works fine too. But the files currently in the patch is named e.g. `sleefinline_advsimd.h`, which does not indicate any platform at all. Is it a generic file, and the platform specific ones are still missing from this PR?
>
> I think both `sleefinline_advsimd.h` and `sleefinline_sve.h` are specific for arm.
> In the future, on riscv the corresponding file name will be `sleefinline_rvvm1.h`.
> 
> Only `misc.h` is a generic file shared among platforms.

Oh, you mean that the `sve` suffix signals that it is for ARM. I thought you were talking about having a name like `sleefinline_aarch64.h`.

Sure, if the only files generated by sleef are ever .h files, the logic for chosing which to include can be done entirely by `#ifdef`s in the source code. But if there ever needs to be different .c or .cpp files to include in the build, the build system needs to be able to determine automatically if they should be included or included, and that can only be made if the path or the file name includes the CPU moniker. 

Personally, I think it would show good alignment with the prevailing norms in the JDK to also use this way of naming files for .h files. But I confess that for .h files it is more a matter of style, rather than a necessity from the build system.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19185#discussion_r1611964018


More information about the build-dev mailing list