Quantcast

RFR 8150388: Remove SPARC 32-bit support

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR 8150388: Remove SPARC 32-bit support

George Triantafillou
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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

harold seigel
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
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

coleen.phillimore
In reply to this post by George Triantafillou
This looks great but I think we should have the compiler group review
also, since lots of compiler code is changed.
thanks!
Coleen

On 4/7/17 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
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

George Triantafillou
In reply to this post by harold seigel
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
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

Vladimir Kozlov
On 4/7/17 8: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

I still see this code is removed. Did you updated webrev?

>>
>> 2. In atomic_solaris_sparc.hpp: remove lines 245 and 372
>>
>> 3. In prefetch_solaris_sparc.inline.hpp, remove lines 30 and 63

Yes. We don't build JVM with C1 only for 64 bit.

>>
>> 4. In thread_solaris_sparc.hpp, remove line 67.

Yes.

Other compiler code changes seems fine.

Thanks,
Vladimir

>>
>> 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
>>>
>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

Vladimir Kozlov
Hi George,

I noticed that you did not changed code guarded by NOT_LP64() and
LP64_ONLY() macros.

Also the code in src/os_cpu/linux_sparc/vm is not changed.

Vladimir

On 4/7/17 10:26 AM, Vladimir Kozlov wrote:

> On 4/7/17 8: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
>
> I still see this code is removed. Did you updated webrev?
>
>>>
>>> 2. In atomic_solaris_sparc.hpp: remove lines 245 and 372
>>>
>>> 3. In prefetch_solaris_sparc.inline.hpp, remove lines 30 and 63
>
> Yes. We don't build JVM with C1 only for 64 bit.
>
>>>
>>> 4. In thread_solaris_sparc.hpp, remove line 67.
>
> Yes.
>
> Other compiler code changes seems fine.
>
> Thanks,
> Vladimir
>
>>>
>>> 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
>>>>
>>>
>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

George Triantafillou
In reply to this post by Vladimir Kozlov
Hi Vladimir,

Thanks for your comments.

Updated webrev:
http://cr.openjdk.java.net/~gtriantafill/8150388-webrev/webrev.01/ 
<http://cr.openjdk.java.net/%7Egtriantafill/8150388-webrev/webrev.01/>

-George

On 4/7/2017 1:26 PM, Vladimir Kozlov wrote:

> On 4/7/17 8: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
>
> I still see this code is removed. Did you updated webrev?
>
>>>
>>> 2. In atomic_solaris_sparc.hpp: remove lines 245 and 372
>>>
>>> 3. In prefetch_solaris_sparc.inline.hpp, remove lines 30 and 63
>
> Yes. We don't build JVM with C1 only for 64 bit.
>
>>>
>>> 4. In thread_solaris_sparc.hpp, remove line 67.
>
> Yes.
>
> Other compiler code changes seems fine.
>
> Thanks,
> Vladimir
>
>>>
>>> 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
>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

George Triantafillou
In reply to this post by Vladimir Kozlov
Hi Vladimir,

On 4/7/2017 1:34 PM, Vladimir Kozlov wrote:
> Hi George,
>
> I noticed that you did not changed code guarded by NOT_LP64() and
> LP64_ONLY() macros.
>
> Also the code in src/os_cpu/linux_sparc/vm is not changed.
My understanding is that this issue only covers solaris_sparc.

-George

>
> Vladimir
>
> On 4/7/17 10:26 AM, Vladimir Kozlov wrote:
>> On 4/7/17 8: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
>>
>> I still see this code is removed. Did you updated webrev?
>>
>>>>
>>>> 2. In atomic_solaris_sparc.hpp: remove lines 245 and 372
>>>>
>>>> 3. In prefetch_solaris_sparc.inline.hpp, remove lines 30 and 63
>>
>> Yes. We don't build JVM with C1 only for 64 bit.
>>
>>>>
>>>> 4. In thread_solaris_sparc.hpp, remove line 67.
>>
>> Yes.
>>
>> Other compiler code changes seems fine.
>>
>> Thanks,
>> Vladimir
>>
>>>>
>>>> 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
>>>>>
>>>>
>>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

Vladimir Kozlov
We will never support 32-bit VM in future JDK releases on all SPARC
platforms which includes Linux (we only build 64 bit JDK on it)! So you
need to clean it too.

Vladimir

On 4/7/17 12:21 PM, George Triantafillou wrote:

> Hi Vladimir,
>
> On 4/7/2017 1:34 PM, Vladimir Kozlov wrote:
>> Hi George,
>>
>> I noticed that you did not changed code guarded by NOT_LP64() and
>> LP64_ONLY() macros.
>>
>> Also the code in src/os_cpu/linux_sparc/vm is not changed.
> My understanding is that this issue only covers solaris_sparc.
>
> -George
>>
>> Vladimir
>>
>> On 4/7/17 10:26 AM, Vladimir Kozlov wrote:
>>> On 4/7/17 8: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
>>>
>>> I still see this code is removed. Did you updated webrev?
>>>
>>>>>
>>>>> 2. In atomic_solaris_sparc.hpp: remove lines 245 and 372
>>>>>
>>>>> 3. In prefetch_solaris_sparc.inline.hpp, remove lines 30 and 63
>>>
>>> Yes. We don't build JVM with C1 only for 64 bit.
>>>
>>>>>
>>>>> 4. In thread_solaris_sparc.hpp, remove line 67.
>>>
>>> Yes.
>>>
>>> Other compiler code changes seems fine.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>>
>>>>> 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
>>>>>>
>>>>>
>>>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

David Holmes
In reply to this post by George Triantafillou
Hi George,

On 8/04/2017 5:19 AM, George Triantafillou wrote:
> Hi Vladimir,
>
> Thanks for your comments.
>
> Updated webrev:
> http://cr.openjdk.java.net/~gtriantafill/8150388-webrev/webrev.01/
> <http://cr.openjdk.java.net/%7Egtriantafill/8150388-webrev/webrev.01/>

There are also changes needed in shared code:

./vm/utilities/globalDefinitions_sparcWorks.hpp**
./vm/utilities/globalDefinitions_gcc.hpp

** which could also do with some Solaris 8 clean out!

Thanks,
David

> -George
>
> On 4/7/2017 1:26 PM, Vladimir Kozlov wrote:
>> On 4/7/17 8: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
>>
>> I still see this code is removed. Did you updated webrev?
>>
>>>>
>>>> 2. In atomic_solaris_sparc.hpp: remove lines 245 and 372
>>>>
>>>> 3. In prefetch_solaris_sparc.inline.hpp, remove lines 30 and 63
>>
>> Yes. We don't build JVM with C1 only for 64 bit.
>>
>>>>
>>>> 4. In thread_solaris_sparc.hpp, remove line 67.
>>
>> Yes.
>>
>> Other compiler code changes seems fine.
>>
>> Thanks,
>> Vladimir
>>
>>>>
>>>> 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
>>>>>
>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

George Triantafillou
In reply to this post by George Triantafillou
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
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

harold seigel
Hi George,

The changes look good.   Thanks for all the work.

Harold


On 4/11/2017 2:09 PM, 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
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

Vladimir Kozlov
In reply to this post by George Triantafillou
In globalDefinitions_gcc.hpp next changes are strange - it is not related to SPARC. 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
>>>>
>>>
>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

David Holmes
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".

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
>>>>>
>>>>
>>>
>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

coleen.phillimore


On 4/11/17 5: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".

This is already a massive change and the bug title says SPARC.   I think
file another RFE for the solaris x86 work.

2284 lines changed: 0 ins; 2231 del; 53 mod; 49567 unchg



Thanks,
Coleen

>
> 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
>>>>>>
>>>>>
>>>>
>>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

Vladimir Kozlov
In reply to this post by David Holmes
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.

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
>>>>>>
>>>>>
>>>>
>>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

David Holmes
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
>>>>>>>
>>>>>>
>>>>>
>>>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

George Triantafillou
In reply to this post by harold seigel
Hi Harold,

Thanks for the review.

-George

On 4/11/2017 2:49 PM, harold seigel wrote:

> Hi George,
>
> The changes look good.   Thanks for all the work.
>
> Harold
>
>
> On 4/11/2017 2:09 PM, 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
>>>>>
>>>>
>>>
>>
>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

George Triantafillou
In reply to this post by David Holmes
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.

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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR 8150388: Remove SPARC 32-bit support

coleen.phillimore
I say ship it!  and file the RFE for removing 32 bit solaris x86 support.
Coleen

On 4/12/17 9: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.
>
> 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
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>

12
Loading...