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

Coleen Phillimore coleen.phillimore at oracle.com
Wed May 21 22:58:56 UTC 2014


I agree.  This is a nice change.
Coleen

On 5/21/14, 4:54 PM, Vladimir Kozlov wrote:
> Yes, this looks good.
>
> Thanks,
> Vladimir
>
> On 5/21/14 1:12 PM, Lois Foltan wrote:
>>
>> On 5/21/2014 2:32 PM, Vladimir Kozlov wrote:
>>> On 5/21/14 11:03 AM, Lois Foltan wrote:
>>>>
>>>> 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.
>>>
>>> Yes, it is a pain to find unique name. Using namespace could solve
>>> this too but we are not using it in VM :(
>>>
>>> So we have duplicated code in these files. In this case can you move
>>> copysignA, scalbnA and other common definitions into .hpp file?
>>
>> Hi Vladimir,
>>
>> Good idea, please review updated webrev at
>> http://cr.openjdk.java.net/~lfoltan/bug_jdk8043301_2/.  I have verified
>> that this builds with VS2010 and VS2013. Retesting is in progress.
>>
>> Thanks,
>> Lois
>>
>>>
>>>>
>>>> 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?
>>>
>>> CC to ppc-aix-port-dev should get their attention. If they will not
>>> comment, we will assume it is okay to change.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>
>>>> 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
>>>>>>>
>>>>>>
>>>>
>>



More information about the hotspot-runtime-dev mailing list