Seemingly contradictory code in sharedRuntime.cpp

[Julian Waters]

Hi David,

Great analysis! That certainly helped clear things up a lot. I wonder
if there are any Windows ARM64 maintainers that we could flag down to
ask about this. The only maintainer of that port that I know of is
Monica (At least I remember she's one of the maintainers)

best regards,
Julian

On Fri, Oct 18, 2024 at 2:34 PM David Holmes <david.holmes at oracle.com> wrote:
>
> On 18/10/2024 2:49 pm, Julian Waters wrote:
> > Hi David,
> >
> > Thanks for pointing that out. This means that something interesting is
> > happening, as the code that is protected by the ifdef block has a
> > comment suggesting that it is solely meant for Windows x64, and not
> > any 64 bit Windows platform that could include ARM64. This possibly
> > means that the ARM64 implementation is using the x64 implementation,
> > with unknown consequences
>
> Possibly ... but cpu/aarch64/c1_LIRGenerator_aarch64.cpp sets up the
> called to SharedRuntime::frem so I think it is okay. I'm assuming the
> folk that did the Windows Aarch64 port would have looked at this.
>
> Looking at:
>
> https://bugs.openjdk.org/browse/JDK-8302191
>
> which introduced the original set of ifdefs:
>
> 1. The original frem was unconditionally used for all platforms
>
> 2. They added an x86 specific version in sharedRuntime_x86.cpp:
>
> #if defined(TARGET_COMPILER_gcc) && !defined(_WIN64)
> JRT_LEAF(jfloat, SharedRuntime::frem(jfloat x, jfloat y))
>
> which loosely implies it would be used for 32-bit or 64-bit Linux (as
> other platforms don't use gcc by default, and for some reason WIN64 was
> explicitly listed).
>
> 3. So they guarded the shared version by the inverse: not x86, or !gcc,
> or else WIN64 - which seems correct as far as it goes.
>
> But then https://bugs.openjdk.org/browse/JDK-8314056
>
> 1. Made the x86 version unconditional - i.e. now used for all 32-bit and
> 64-bit x86 platforms
>
> 2. So they removed the additional ifdefs from the shared version so that
> it was now only for !X86.
>
> That also seems correct in isolation, but that then leaves the _WIN64
> ifdef'd code within the shared version as dead code from a Windows
> x86_64 perspective. But it is still used for Windows Aarch64 ... which
> is possibly harmless, but I doubt this actually applies:
>
>   // 64-bit Windows on amd64 returns the wrong values for
>   // infinity operands.
>
> so potentially Windows Aarch64 may be able to use "better" code?
>
> Anyway - good find!
>
> Cheers,
> David
>
> > best regards,
> > Julian
>