RFR: 8282392: [zero] Build broken on AArch64 [v2]
Andrew Haley
aph at openjdk.java.net
Tue Mar 1 09:45:14 UTC 2022
On Mon, 28 Feb 2022 16:48:35 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>>> I think it is confusing to have `AARCH64_PORT_ONLY` defines, to be honest. In the similar cases for X86, we just additionally protect these blocks with !ZERO. Something like:
>>
>> That's what we looked at and it was more of a mess, IMO. In the end it's a judgment call which to have, and I've seen this kind of mistake, where a particular port is confused with a particular CPU, enough times that I think this is OK; YMMV.
>
>> That's what we looked at and it was more of a mess, IMO. In the end it's a judgment call which to have, and I've seen this kind of mistake, where a particular port is confused with a particular CPU, enough times that I think this is OK; YMMV.
>
> From the perspective of Zero maintenance, having the Zero-specific workarounds explicitly doing `!ZERO` is cleaner. This mess is mostly Zero-s problem with idenitifying itself as CPU. So, in my mind, there is little reason to accommodate that problem with "port" defines.
> Sorry I missed this but this is stylistically awful IMHO. What is AARCH64_PORT_ONLY supposed to mean? IIUC it really means AARCH64_NATIVE_PORT_ONLY or AARCH64_NOT_ZERO_ONLY. I would much rather have seen @shipilev request to use a combination of CPU and ZERO to get this right - as used everywhere else when dealing with ZERO. And we don't use TARGET_ARCH_x anywhere else in the code other than to define the macros.
I've seen this problem repeating over the years, caused by a failure to distinguish between an ifdef for the properties of a particular CPU and one for the implementation of a particular port. Courtesy of Zero, there is not a 1:1 mapping between these, and (judging by the problems we've seen) it is a frequent cause of confusion. Usually we catch the bugs before push, but not always.
In my opinion that this is not merely a matter of people making mistakes, but of a style that is confusing, and will continue to be.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7633
More information about the hotspot-dev
mailing list