RFR(S): 8210676: Remove some unused Label variables
Please review this change which removes a bunch of unused Label variables. Would appreciate some help from aarch64/ppc/s390x folks to verify it! bug: https://bugs.openjdk.java.net/browse/JDK-8210676 webrev: http://cr.openjdk.java.net/~mikael/webrevs/8210676/webrev.03/open/webrev/ * Background (from bug) [~dholmes] noticed during the code review of JDK-8210381 that the "Label Egress" variable in macroAssembler_sparc.cpp was unused. It and other unused labels like it should be removed. * About the change I have *not* tried to find and remove *all* unused Label variables, because that turns out to be much harder than it might seem. I may or may not follow up on this work to remove additional unused Label variables later, but before that I’m investigating removal of other unused variables in general. Meanwhile I like to think that this is a reasonable cleanup anyway. * Testing tier1 build&test passes. Cheers, Mikael
Looks good to me. Thanks, Vladimir On 9/13/18 4:03 PM, Mikael Vidstedt wrote:
Please review this change which removes a bunch of unused Label variables. Would appreciate some help from aarch64/ppc/s390x folks to verify it!
bug: https://bugs.openjdk.java.net/browse/JDK-8210676 webrev: http://cr.openjdk.java.net/~mikael/webrevs/8210676/webrev.03/open/webrev/
* Background (from bug)
[~dholmes] noticed during the code review of JDK-8210381 that the "Label Egress" variable in macroAssembler_sparc.cpp was unused. It and other unused labels like it should be removed.
* About the change
I have *not* tried to find and remove *all* unused Label variables, because that turns out to be much harder than it might seem. I may or may not follow up on this work to remove additional unused Label variables later, but before that I’m investigating removal of other unused variables in general. Meanwhile I like to think that this is a reasonable cleanup anyway.
* Testing
tier1 build&test passes.
Cheers, Mikael
Looks good - though the proof is in the building of course. The debug-only use of notDouble in src/hotspot/cpu/x86/templateTable_x86.cpp seems somewhat odd. Thanks, David On 14/09/2018 9:03 AM, Mikael Vidstedt wrote:
Please review this change which removes a bunch of unused Label variables. Would appreciate some help from aarch64/ppc/s390x folks to verify it!
bug: https://bugs.openjdk.java.net/browse/JDK-8210676 webrev: http://cr.openjdk.java.net/~mikael/webrevs/8210676/webrev.03/open/webrev/
* Background (from bug)
[~dholmes] noticed during the code review of JDK-8210381 that the "Label Egress" variable in macroAssembler_sparc.cpp was unused. It and other unused labels like it should be removed.
* About the change
I have *not* tried to find and remove *all* unused Label variables, because that turns out to be much harder than it might seem. I may or may not follow up on this work to remove additional unused Label variables later, but before that I’m investigating removal of other unused variables in general. Meanwhile I like to think that this is a reasonable cleanup anyway.
* Testing
tier1 build&test passes.
Cheers, Mikael
On Sep 13, 2018, at 6:17 PM, David Holmes <david.holmes@oracle.com> wrote:
Looks good - though the proof is in the building of course.
The debug-only use of notDouble in src/hotspot/cpu/x86/templateTable_x86.cpp seems somewhat odd.
Would you prefer to see it declared next to the other labels, but guarded with ASSERT? Cheers, Mikael
Thanks, David
On 14/09/2018 9:03 AM, Mikael Vidstedt wrote:
Please review this change which removes a bunch of unused Label variables. Would appreciate some help from aarch64/ppc/s390x folks to verify it! bug: https://bugs.openjdk.java.net/browse/JDK-8210676 webrev: http://cr.openjdk.java.net/~mikael/webrevs/8210676/webrev.03/open/webrev/ * Background (from bug) [~dholmes] noticed during the code review of JDK-8210381 that the "Label Egress" variable in macroAssembler_sparc.cpp was unused. It and other unused labels like it should be removed. * About the change I have *not* tried to find and remove *all* unused Label variables, because that turns out to be much harder than it might seem. I may or may not follow up on this work to remove additional unused Label variables later, but before that I’m investigating removal of other unused variables in general. Meanwhile I like to think that this is a reasonable cleanup anyway. * Testing tier1 build&test passes. Cheers, Mikael
On 14/09/2018 11:22 AM, Mikael Vidstedt wrote:
On Sep 13, 2018, at 6:17 PM, David Holmes <david.holmes@oracle.com> wrote:
Looks good - though the proof is in the building of course.
The debug-only use of notDouble in src/hotspot/cpu/x86/templateTable_x86.cpp seems somewhat odd.
Would you prefer to see it declared next to the other labels, but guarded with ASSERT?
No what you have is fine. I'm just curious about why the double case is only examined in a non-product build ?? David
Cheers, Mikael
Thanks, David
On 14/09/2018 9:03 AM, Mikael Vidstedt wrote:
Please review this change which removes a bunch of unused Label variables. Would appreciate some help from aarch64/ppc/s390x folks to verify it! bug: https://bugs.openjdk.java.net/browse/JDK-8210676 webrev: http://cr.openjdk.java.net/~mikael/webrevs/8210676/webrev.03/open/webrev/ * Background (from bug) [~dholmes] noticed during the code review of JDK-8210381 that the "Label Egress" variable in macroAssembler_sparc.cpp was unused. It and other unused labels like it should be removed. * About the change I have *not* tried to find and remove *all* unused Label variables, because that turns out to be much harder than it might seem. I may or may not follow up on this work to remove additional unused Label variables later, but before that I’m investigating removal of other unused variables in general. Meanwhile I like to think that this is a reasonable cleanup anyway. * Testing tier1 build&test passes. Cheers, Mikael
Hi Mikael, AArch64 and Arm parts looks good and I have also verified the build. Thanks, Ningsheng
-----Original Message----- From: aarch64-port-dev <aarch64-port-dev-bounces@openjdk.java.net> On Behalf Of Mikael Vidstedt Sent: Friday, September 14, 2018 7:03 AM To: HotSpot Open Source Developers <hotspot-dev@openjdk.java.net> Cc: s390x-port-dev@openjdk.java.net; ppc-aix-port-dev@openjdk.java.net; aarch64-port-dev@openjdk.java.net Subject: [aarch64-port-dev ] RFR(S): 8210676: Remove some unused Label variables
Please review this change which removes a bunch of unused Label variables. Would appreciate some help from aarch64/ppc/s390x folks to verify it!
bug: https://bugs.openjdk.java.net/browse/JDK-8210676 webrev: http://cr.openjdk.java.net/~mikael/webrevs/8210676/webrev.03/open/webrev/
* Background (from bug)
[~dholmes] noticed during the code review of JDK-8210381 that the "Label Egress" variable in macroAssembler_sparc.cpp was unused. It and other unused labels like it should be removed.
* About the change
I have *not* tried to find and remove *all* unused Label variables, because that turns out to be much harder than it might seem. I may or may not follow up on this work to remove additional unused Label variables later, but before that I’m investigating removal of other unused variables in general. Meanwhile I like to think that this is a reasonable cleanup anyway.
* Testing
tier1 build&test passes.
Cheers, Mikael
participants (4)
-
David Holmes
-
Mikael Vidstedt
-
Ningsheng Jian (Arm Technology China)
-
Vladimir Kozlov