Seemingly contradictory code in sharedRuntime.cpp
Vladimir Ivanov
vladimir.x.ivanov at oracle.com
Thu Oct 17 23:15:32 UTC 2024
> The issue was introduced by [1] changes. I think it is incorrectly
> removed `defined(_WIN64)` from `#if` in sharedRuntime.cpp
It looks like _WIN64-specific code in sharedRuntime.cpp is redundant
now. There's x86-specific implementation of SharedRuntime::frem in
sharedRuntime_x86.cpp which is not guarded by "!defined(_WIN64)" anymore.
Best regards,
Vladimir Ivanov
> [1]
> https://github.com/openjdk/jdk/commit/ce2a7ea40a22c652e5f8559c91d5eea197e2d708#diff-808315795bdfbd3d48d1665603fc2fe0db416fa299394371f24dbed2cb32caa4
>
> On 10/17/24 8:28 AM, Julian Waters wrote:
>> Hi all,
>>
>> Recently while compiling the Windows JDK, gcc caught a very strange
>> case in sharedRuntime.cpp. The SharedRuntime::frem and
>> SharedRuntime::drem methods both contain code that is only compiled if
>> _WIN64 is defined. The problem is this is then surrounded by an ifndef
>> check for X86. HotSpot defines X86 if AMD64 or IA32 is defined. Since
>> IA32 is not a supported architecture, only AMD64 is of interest to us
>> here. But AMD64 and _WIN64 will always both be defined at the same
>> time, as _WIN64 is an AMD64 platform. So by extension, X86 and _WIN64
>> will always be defined together. But remember that the Windows
>> specific code in frem and drem will only be compiled if X86 is not
>> defined and _WIN64 is defined. This means the block of code that is
>> guarded by the _WIN64 define is never ever compiled. To add to that,
>> sharedRuntime_x86.cpp also contains alternate method definitions for
>> SharedRuntime::frem and SharedRuntime::drem. This all leads me to
>> believe it is a bug where the code meant for Windows x64 was
>> erroneously added to the implementation for non x86 platforms instead
>> of the correct one inside sharedRuntime_x86.cpp. I just thought I'd
>> give everyone a heads up, and also to confirm that this is indeed a
>> bug, before I start working on a fix
>>
>> The code in question:
>>
>> #if !defined(X86) // <----- This will always be defined if _WIN64 is
>> defined!
>> JRT_LEAF(jfloat, SharedRuntime::frem(jfloat x, jfloat y))
>> #ifdef _WIN64 // <----- This will always be defined if X86 is defined!
>> // 64-bit Windows on amd64 returns the wrong values for
>> // infinity operands.
>> juint xbits = PrimitiveConversions::cast<juint>(x);
>> juint ybits = PrimitiveConversions::cast<juint>(y);
>> // x Mod Infinity == x unless x is infinity
>> if (((xbits & float_sign_mask) != float_infinity) &&
>> ((ybits & float_sign_mask) == float_infinity) ) {
>> return x;
>> }
>> return ((jfloat)fmod_winx64((double)x, (double)y));
>> #else
>> return ((jfloat)fmod((double)x,(double)y));
>> #endif
>> JRT_END
>>
>> JRT_LEAF(jdouble, SharedRuntime::drem(jdouble x, jdouble y))
>> #ifdef _WIN64 <----- This will always be defined if X86 is defined!
>> julong xbits = PrimitiveConversions::cast<julong>(x);
>> julong ybits = PrimitiveConversions::cast<julong>(y);
>> // x Mod Infinity == x unless x is infinity
>> if (((xbits & double_sign_mask) != double_infinity) &&
>> ((ybits & double_sign_mask) == double_infinity) ) {
>> return x;
>> }
>> return ((jdouble)fmod_winx64((double)x, (double)y));
>> #else
>> return ((jdouble)fmod((double)x,(double)y));
>> #endif
>> JRT_END
>> #endif // !X86
>>
>> best regards,
>> Julian
>
More information about the hotspot-dev
mailing list