RFR: 8311130: AArch64: Sync SVE related CPU features with VM options [v2]
Andrew Haley
aph at openjdk.org
Tue Jul 18 09:53:13 UTC 2023
On Tue, 18 Jul 2023 09:01:29 GMT, Pengfei Li <pli at openjdk.org> wrote:
>> As discussed in PR #14533, keeping AArch64 flag `UseSVE` and its related CPU features in sync helps to simplify rules in IR tests. In this patch, we mask SVE related CPU features off if specified SVE level in VM option is lower than the hardware supported. Also, to support this change, we move the features string construction to the end of the `initialize()` function.
>>
>> We also revert IR rule changes in PR #14533 and fix some code styles. We tested almost full jtreg on SVE, SVE2 and non-SVE CPUs and no new issue is found after this patch.
>
> Pengfei Li has updated the pull request incrementally with two additional commits since the last revision:
>
> - Simplify some checks
> - Address aph's comment
Looks good, but please fix the dangling else.
src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 580:
> 578: int buf_used_len = os::snprintf_checked(buf, sizeof(buf), "0x%02x:0x%x:0x%03x:%d", _cpu, _variant, _model, _revision);
> 579: if (_model2) os::snprintf_checked(buf + buf_used_len, sizeof(buf) - buf_used_len, "(0x%03x)", _model2);
> 580: #define ADD_FEATURE_IF_SUPPORTED(id, name, bit) if (VM_Version::supports_##name()) strcat(buf, ", " #name);
```suggestion \
#define ADD_FEATURE_IF_SUPPORTED(id, name, bit) \
do { \
if (VM_Version::supports_##name()) strcat(buf, ", " #name); \
while(0);
Reason: no dangling else in macros, ever.
-------------
PR Review: https://git.openjdk.org/jdk/pull/14897#pullrequestreview-1534658549
PR Review Comment: https://git.openjdk.org/jdk/pull/14897#discussion_r1266528503
More information about the hotspot-dev
mailing list