RFR: 8366777: Build fails unknown pseudo-op with old AS on linux-aarch64 [v18]
Erik Joelsson
erikj at openjdk.org
Fri Sep 12 16:33:24 UTC 2025
On Fri, 12 Sep 2025 14:51:07 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:
>> make/autoconf/flags-other.m4 line 120:
>>
>>> 118: DESC: [Use SVE when compiling libsleef],
>>> 119: AVAILABLE: false,
>>> 120: CHECK_AVAILABLE: [
>>
>> 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)
>
> Oh, and also, you can extract a common:
>
> AC_MSG_RESULT([$AARCH64_SVE_AVAILABLE])
>
> after the compile test. This lets you get rid of the else block completely, and it makes it clear that the AC_MSG_CHECKING is always terminated. (Apart from being shorter)
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27073#discussion_r2344815625
More information about the build-dev
mailing list