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

Andrew Haley aph at openjdk.org
Wed Sep 14 08:58:44 UTC 2022


On Tue, 13 Sep 2022 10:28:43 GMT, Olga Mikhaltsova <omikhaltcova at openjdk.org> wrote:

>> src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot.aarch64/src/jdk/vm/ci/hotspot/aarch64/AArch64HotSpotRegisterConfig.java line 291:
>> 
>>> 289:                     currentStackOffset += Math.max(valueKind.getPlatformKind().getSizeInBytes(), target.wordSize);
>>> 290:                 }
>>> 291:             }
>> 
>> So I'm curious: why not subclass `AArch64HotSpotRegisterConfig` here, or maybe even use an interface, rather than the boolean?
>
> 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 fot MacOS and one for the others. I  might be wrong, but I'd try it.

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

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


More information about the hotspot-compiler-dev mailing list