RFR (S) JDK-8043301: Duplicate definitions in vm/runtime/sharedRuntimeTrans.cpp versus math.h in VS2013

[Lois Foltan]

On 5/21/2014 1:47 PM, Vladimir Kozlov wrote:
> These methods are there to get the same results on all platforms.
>
> I don't like adding 'A'. Can we use __scalbn names?
>
> Also we need to ask SAP if they have conflict with new names before 
> removing #if.

Thanks Vladimir for the review.  I hesitate to go to the use of double 
underscores since I thought this was reserved for a C/C++ compiler's 
internal use.   Also one underscore is how copysign is defined within 
VS2010's math.h so that is not a viable option either.  Again, there was 
precedent within sharedRuntimeTrig.cpp for the 'A', so I chose to follow 
suit.

I would be happy to follow up with SAP, do you have a specific contact, 
or by cc'ing the ppc-aix-port-dev alias that should be sufficient?

Lois

> Thanks,
> Vladimir
>
> On 5/21/14 10:22 AM, Lois Foltan wrote:
>>
>> On 5/21/2014 12:49 PM, Tim Bell wrote:
>>> Lois:
>>>
>>>> Please review the following fix:
>>>>
>>>> Webrev:
>>>>     http://cr.openjdk.java.net/~lfoltan/bug_jdk8043301/
>>>>
>>>> Bug: Duplicate definitions in vm/runtime/sharedRuntimeTrans.cpp
>>>> versus math.h in VS2013
>>>>     https://bugs.openjdk.java.net/browse/JDK-8043301
>>>>
>>>> Summary of fix:
>>>> VS2013's include/math.h now contains definitions for both copysign()
>>>> and scalbn().  This yielded compilation conflicts with the JVM's
>>>> definition of these functions within runtime/sharedRuntimeTrans.cpp.
>>>> I changed copysign and scalbn definitions to be consistent with how
>>>> they are defined within sharedRuntimeTrig.cpp.  They are now defined
>>>> as copysignA and scalbnA static functions.  I also removed the
>>>> !defined(AIX) conditionalization around these defintions since no
>>>> conflict should now exist with AIX's math.h.
>>>>
>>>> Tests:
>>>>     JPRT build & test, Hotspot jtreg, JDK lang & util,
>>>> vm.quick.testlist (in progress)
>>>
>>> Thank you for the fix.  Looks good to me, although I am not a hotspot
>>> reviewer or committer.  I have very low confidence that their
>>> implementations of 'copysign' and 'scalbn' would exactly match what we
>>> use on all other platforms, especially since we can't inspect their 
>>> code.
>>
>> Thanks Tim for the review.  I think due to performance reasons
>> sharedRuntimeTrans.cpp and sharedRuntimeTrig.cpp have their own
>> definitions.   These are the only two files in the JVM that use copysign
>> and scalbn.
>>
>>>
>>> I have the sharedRuntimeTrans.cpp patch in my compiler upgrade source
>>> tree... testing now.
>>
>> Great, thanks again!
>> Lois
>>
>>>
>>> Tim
>>>
>>