RFR: 8262901: [macos_aarch64] NativeCallTest expected:<-3.8194101E18> but was:<3.02668882E10> [v3]

Olga Mikhaltsova omikhaltcova at openjdk.org
Mon Nov 7 13:13:27 UTC 2022


On Wed, 14 Sep 2022 08:56:24 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> I tried to be closer to the original review https://github.com/openjdk/jdk/pull/6641 that requires only 2 fixes and tried to do only this in order to continue easily.
>> 
>> Could you clarify please what boolean you talk about? `private final boolean macOS;` that was pushed into `class AArch64HotSpotRegisterConfig`, right? I'm hesitating a bit because of the highlighted code.
>
> Yes, that `macOS` boolean.
> 
> Maybe it's not worth the effort, but it seems to me as though the use of the boolean in several places is something of a code smell, and this patch makes it more so. The control flow is not easy to follow.
> I am wondering if refactoring it so that the code between L269 and L291 were broken out into two methods, one for MacOS and one for the others. I  might be wrong, but I'd try it.

@theRealAph could you please take a look! I've made some additional refactoring in order to get rid of those "boolean"s where it's possible. I moved them from AArch64HotSpotVMConfig to TargetDescription. This makes an access to them easier across the code. As a 2nd step it's possible to substitute these "boolean"s with "enum" if it's needed. But imho it's better to keep them as "boolean"s at the moment.
In addition I see a new class RISCV64HotSpotVMConfig (committed after this pr) that also declared the same boolean:
`final boolean linuxOs = Services.getSavedProperty("os.name", "").startsWith("Linux");`
If this refactoring is made, this "boolean" won't be needed as well because it can be accessed from TargetDescription.

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

PR: https://git.openjdk.org/jdk/pull/10238


More information about the hotspot-compiler-dev mailing list