RFR 8150388: Remove SPARC 32-bit support
Jini George
jini.george at oracle.com
Sat Apr 15 16:31:37 UTC 2017
Hi George,
Would you be making the corresponding SA changes too or filing a
separate RFE for removing the SPARC-32 bit support from SA ?
Thanks,
Jini.
On 4/13/2017 1:53 AM, George Triantafillou wrote:
> Thanks David, Vladimir, Harold and Coleen for the reviews.
>
> -George
>
> On 4/12/2017 4:20 PM, David Holmes wrote:
>> Agree. Sorry for the mis-steer on this.
>>
>> That RFE should be for removal of all remaining Solaris 32-bit support:
>> - x86
>> - shared
>> - build
>> - test
>> ...
>>
>> :)
>>
>> Thanks,
>> David
>>
>> On 13/04/2017 2:10 AM, Vladimir Kozlov wrote:
>>> On 4/12/17 6:26 AM, George Triantafillou wrote:
>>>> Hi David and Vladimir,
>>>>
>>>> If everyone is in agreement, I'll revert the changes to
>>>> src/share/vm/utilities/globalDefinitions_sparcWorks.hpp and
>>>> src/share/vm/utilities/globalDefinitions_gcc.hpp and push the fix to
>>>> jdk10-hs.
>>>
>>> Agree.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>
>>>> Thanks.
>>>>
>>>> -George
>>>>
>>>> On 4/11/2017 10:40 PM, David Holmes wrote:
>>>>> On 12/04/2017 10:17 AM, Vladimir Kozlov wrote:
>>>>>> On 4/11/17 2:39 PM, David Holmes wrote:
>>>>>>> On 12/04/2017 5:23 AM, Vladimir Kozlov wrote:
>>>>>>>> In globalDefinitions_gcc.hpp next changes are strange - it is not
>>>>>>>> related to SPARC.
>>>>>>>
>>>>>>> So ... why is this restricted to 32-bit sparc and not simply 32-bit
>>>>>>> Solaris ?? The changes I pointed out needed for
>>>>>>> shared code are really about 32-bit Solaris not specific to
>>>>>>> sparc. If
>>>>>>> this is really intended to only deal with sparc
>>>>>>> then those changes should probably be left off until we do the
>>>>>>> remaining "remove solaris 32-bit support".
>>>>>>
>>>>>> globalDefinitions_gcc.hpp is not Solaris specific.
>>>>>> First change in the file is guarded by #ifdef SOLARIS and I am fine
>>>>>> with
>>>>>> it. But 2nd and 3rd are not guarded by it - it can affect Linux too.
>>>>>> Also 4th change is APPLE specific.
>>>>>
>>>>> Yes those changes are wrong.
>>>>>
>>>>> Similarly in globalDefinitions_sparcWorks.hpp there are changes
>>>>> inside LINUX defined blocks that should not be made, and a change to
>>>>> platform agnostic defines, that should not be made.
>>>>>
>>>>> But it may be best for George to simply drop those files and keep
>>>>> this changeset to being SPARC specific. Though I hope we already have
>>>>> RFE's filed for the remaining Solaris 32-bit cleanup.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Vladimir
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> We still support 32-bit linux on x86 (I agree with
>>>>>>>> with last APPLE change).
>>>>>>>>
>>>>>>>> #ifdef __GNUC__
>>>>>>>> - #ifdef _LP64
>>>>>>>> #define NULL_WORD 0L
>>>>>>>> - #else
>>>>>>>> - // Cast 0 to intptr_t rather than int32_t since they are
>>>>>>>> not the
>>>>>>>> same type
>>>>>>>> - // on platforms such as Mac OS X.
>>>>>>>> - #define NULL_WORD ((intptr_t)0)
>>>>>>>> - #endif
>>>>>>>> #else
>>>>>>>>
>>>>>>>>
>>>>>>>> // Formatting.
>>>>>>>> -#ifdef _LP64
>>>>>>>> #define FORMAT64_MODIFIER "l"
>>>>>>>> -#else // !_LP64
>>>>>>>> -#define FORMAT64_MODIFIER "ll"
>>>>>>>> -#endif // _LP64
>>>>>>>>
>>>>>>>>
>>>>>>>> Otherwise changes are good.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Vladimir
>>>>>>>>
>>>>>>>> On 4/11/17 11:09 AM, George Triantafillou wrote:
>>>>>>>>> Here's an updated webrev addressing changes from Vladimir,
>>>>>>>>> Harold, and
>>>>>>>>> David:
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~gtriantafill/8150388-webrev/webrev.03/
>>>>>>>>>
>>>>>>>>> <http://cr.openjdk.java.net/%7Egtriantafill/8150388-webrev/webrev.03/>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks.
>>>>>>>>>
>>>>>>>>> -George
>>>>>>>>>
>>>>>>>>> On 4/7/2017 11:04 AM, George Triantafillou wrote:
>>>>>>>>>> Thanks Harold, I've made the suggested changes.
>>>>>>>>>>
>>>>>>>>>> -George
>>>>>>>>>>
>>>>>>>>>> On 4/7/2017 10:34 AM, harold seigel wrote:
>>>>>>>>>>> Hi George,
>>>>>>>>>>>
>>>>>>>>>>> Most of the changes look good. A few comments:
>>>>>>>>>>>
>>>>>>>>>>> 1. In stubGenerator_sparc.cpp, lines 831-835 should not be
>>>>>>>>>>> deleted,
>>>>>>>>>>> just the "&& defined(_LP64)"
>>>>>>>>>>>
>>>>>>>>>>> 831 #if defined(ASSERT) && defined(_LP64)
>>>>>>>>>>> 832 __ signx(Rint, Rtmp);
>>>>>>>>>>> 833 __ cmp(Rint, Rtmp);
>>>>>>>>>>> 834 __ breakpoint_trap(Assembler::notEqual, Assembler::xcc);
>>>>>>>>>>> 835 #endif
>>>>>>>>>>>
>>>>>>>>>>> 2. In atomic_solaris_sparc.hpp: remove lines 245 and 372
>>>>>>>>>>>
>>>>>>>>>>> 3. In prefetch_solaris_sparc.inline, remove lines 30 and 63
>>>>>>>>>>>
>>>>>>>>>>> 4. In thread_solaris_sparc.hpp, remove line 67.
>>>>>>>>>>>
>>>>>>>>>>> Thanks, Harold
>>>>>>>>>>>
>>>>>>>>>>> On 4/7/2017 9:43 AM, George Triantafillou wrote:
>>>>>>>>>>>> Please review this fix to remove SPARC 32-bit support.
>>>>>>>>>>>> Support for
>>>>>>>>>>>> solaris-sparc has been dropped from the list of
>>>>>>>>>>>> supported platforms.
>>>>>>>>>>>>
>>>>>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8150388
>>>>>>>>>>>> webrev:
>>>>>>>>>>>> http://cr.openjdk.java.net/~gtriantafill/8150388-webrev/webrev/
>>>>>>>>>>>>
>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Egtriantafill/8150388-webrev/webrev/>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Built and tested on solaris-sparcv9-debug,solaris-x64-debug
>>>>>>>>>>>> with
>>>>>>>>>>>> the nsk.jvmti, nsk.jdwp, and nsk.jdi testlists.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>
>>>>>>>>>>>> -George
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>
>
More information about the hotspot-runtime-dev
mailing list