On Fri, 12 Sep 2025 16:30:09 GMT, Erik Joelsson <er...@openjdk.org> wrote:
>>> You are not supposed to both provide AVAILABLE and CHECK_AVAILABLE. Also, >>> if you do setup CHECK_AVAILABLE you should always provide a value to the >>> AVAILABLE variable; now you don't have an else clause for the cpu and >>> toolchain tests. Also, CHECK_AVAILABLE was designed for small and simple >>> tests; this is so long it becames hard to read. >>> >>> Instead I suggest the following: Start by setting AARCH64_SVE_AVAILABLE to >>> false. Then do the code that is currently in the CHECK_AVAILABLE block, but >>> replace the assignment to AVAILABLE with an assignment to >>> AARCH64_SVE_AVAILABLE. (Where it is true, since you defaulted it to false >>> you can skip that line). >>> >>> Then, finally, you can do like this: >>> >>> ``` >>> UTIL_ARG_ENABLE(NAME: aarch64-sve, DEFAULT: auto, >>> RESULT: AARCH64_SVE_ENABLED, >>> DESC: [Use SVE when compiling libsleef], >>> AVAILABLE: $AARCH64_SVE_AVAILABLE) >>> ``` >> >> Here is the code evaluating AVAILABLE: >> >> >> # Check if the option is available >> AVAILABLE=ARG_AVAILABLE >> # Run the available check block (if any), which can overwrite AVAILABLE. >> ARG_CHECK_AVAILABLE >> >> Which is why I interpreted it as ok to supply both AVAILABLE as a default >> and AVAILABLE_CHECK. >> >> Also I don't agree that this is too big to be inline. I think it's perfectly >> readable. > > The issue with using the AVAILABLE variable in `AC_MSG_RESULT` is that it has > the values true/false not yes/no, which is the common output for results of > configure tests. Thanks. CHECK_AVAILABLE has been removed from UTIL_ARG_ENABLE, I think this will make the usage of UTIL_ARG_ENABLE looks more consistent with other makefiles. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27073#discussion_r2347321246
