Quantcast

[8u] RFR: 8177817: Remove assertions in 8u that were removed by 8056124 in 9.

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

[8u] RFR: 8177817: Remove assertions in 8u that were removed by 8056124 in 9.

KEVIN WALLS
Hi,

I'm backporting several sparc/cpu recognition kind of changes into 8u,
and would like to get approval on this change which will help them
backport nicely.

I've got 8043913 and 8049717 into 8u, and next is:

8134119: Use new API to get cache line sizes
https://bugs.openjdk.java.net/browse/JDK-8134119

When I backport 8134119, the assert added by 8049717 can trigger.  In 9
that assert was removed by:

8056124: Hotspot should use PICL interface to get cacheline size on SPARC

...which we have in 8u already, but when 8056124 was taken into 8u that
assert wasn't there.

So for 8u, I simply want to remove those asserts, that is in a new CR,
and this email is a review request:

8177817: Remove assertions in 8u that were removed by 8056124 in 9.
https://bugs.openjdk.java.net/browse/JDK-8177817

It's a simple change which removes some code, which is already removed
in 9 as part of 8056124, and the actual diff I've pasted in below.  It's
all deletions, as they are in the 9 changeset for 8056124 (
http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/63934ec778a2 ).


Then I'll  go ahead and backport 8134119 to 8u.

https://bugs.openjdk.java.net/browse/JDK-8134119
http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/raw-rev/be30670bbd35

That will import except for changing os::strdup_check_oom() as it's not
recognised, we only have os::strdup().  That doesn't require separate
approval here I think, but I'll mention it as these are connected.

(To clarify, I haven't pushed 8049717 where the asset appears into 8u yet. I'd like to push 8049717, and the removal of the asserts in the new CR 8177817, and also 8134119, at the same time.  These latter two will go to the 8u-dev list next...  I could have done the code removal muddled in with the backports, but thought the above was neater.  It would require one visit to this list either way.)

Many thanks!
Kevin



bash-4.2$ hg diff
diff -r 84212a4e67e1 src/cpu/sparc/vm/vm_version_sparc.cpp
--- a/src/cpu/sparc/vm/vm_version_sparc.cpp     Fri Mar 17 07:52:54 2017
-0700
+++ b/src/cpu/sparc/vm/vm_version_sparc.cpp     Thu Mar 30 09:27:20 2017
-0700
@@ -259,49 +259,6 @@
    // buf is started with ", " or is empty
    _features_str = strdup(strlen(buf) > 2 ? buf + 2 : buf);

-  // There are three 64-bit SPARC families that do not overlap, e.g.,
-  // both is_ultra3() and is_sparc64() cannot be true at the same time.
-  // Within these families, there can be more than one chip, e.g.,
-  // is_T4() and is_T7() machines are also is_niagara().
-  if (is_ultra3()) {
-    assert(_L1_data_cache_line_size == 0, "overlap with Ultra3 family");
-    // Ref: UltraSPARC III Cu Processor
-    _L1_data_cache_line_size = 64;
-  }
-  if (is_niagara()) {
-    assert(_L1_data_cache_line_size == 0, "overlap with niagara family");
-    // All Niagara's are sun4v's, but not all sun4v's are Niagaras, e.g.,
-    // Fujitsu SPARC64 is sun4v, but we don't want it in this block.
-    //
-    // Ref: UltraSPARC T1 Supplement to the UltraSPARC Architecture 2005
-    // Appendix F.1.3.1 Cacheable Accesses
-    // -> 16-byte L1 cache line size
-    //
-    // Ref: UltraSPARC T2: A Highly-Threaded, Power-Efficient, SPARC SOC
-    // Section III: SPARC Processor Core
-    // -> 16-byte L1 cache line size
-    //
-    // Ref: Oracle's SPARC T4-1, SPARC T4-2, SPARC T4-4, and SPARC
T4-1B Server Architecture
-    // Section SPARC T4 Processor Cache Architecture
-    // -> 32-byte L1 cache line size (no longer see that info on this ref)
-    //
-    // XXX - still need a T7 reference here
-    //
-    if (is_T7()) {  // T7 or newer
-      _L1_data_cache_line_size = 64;
-    } else if (is_T4()) {  // T4 or newer (until T7)
-      _L1_data_cache_line_size = 32;
-    } else {  // T1 or newer (until T4)
-      _L1_data_cache_line_size = 16;
-    }
-  }
-  if (is_sparc64()) {
-    guarantee(_L1_data_cache_line_size == 0, "overlap with SPARC64
family");
-    // Ref: Fujitsu SPARC64 VII Processor
-    // Section 4 Cache System
-    _L1_data_cache_line_size = 64;
-  }
-
    // UseVIS is set to the smallest of what hardware supports and what
    // the command line requires.  I.e., you cannot set UseVIS to 3 on
    // older UltraSparc which do not support it.


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

Re: [8u] RFR: 8177817: Remove assertions in 8u that were removed by 8056124 in 9.

Igor Veresov
We’ve discussed this privately and I agree with the plan.

igor

> On Mar 30, 2017, at 10:02 AM, Kevin Walls <[hidden email]> wrote:
>
> Hi,
>
> I'm backporting several sparc/cpu recognition kind of changes into 8u, and would like to get approval on this change which will help them backport nicely.
>
> I've got 8043913 and 8049717 into 8u, and next is:
>
> 8134119: Use new API to get cache line sizes
> https://bugs.openjdk.java.net/browse/JDK-8134119
>
> When I backport 8134119, the assert added by 8049717 can trigger.  In 9 that assert was removed by:
>
> 8056124: Hotspot should use PICL interface to get cacheline size on SPARC
>
> ...which we have in 8u already, but when 8056124 was taken into 8u that assert wasn't there.
>
> So for 8u, I simply want to remove those asserts, that is in a new CR, and this email is a review request:
>
> 8177817: Remove assertions in 8u that were removed by 8056124 in 9.
> https://bugs.openjdk.java.net/browse/JDK-8177817
>
> It's a simple change which removes some code, which is already removed in 9 as part of 8056124, and the actual diff I've pasted in below.  It's all deletions, as they are in the 9 changeset for 8056124 ( http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/63934ec778a2 ).
>
>
> Then I'll  go ahead and backport 8134119 to 8u.
>
> https://bugs.openjdk.java.net/browse/JDK-8134119
> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/raw-rev/be30670bbd35
>
> That will import except for changing os::strdup_check_oom() as it's not recognised, we only have os::strdup().  That doesn't require separate approval here I think, but I'll mention it as these are connected.
>
> (To clarify, I haven't pushed 8049717 where the asset appears into 8u yet. I'd like to push 8049717, and the removal of the asserts in the new CR 8177817, and also 8134119, at the same time.  These latter two will go to the 8u-dev list next...  I could have done the code removal muddled in with the backports, but thought the above was neater.  It would require one visit to this list either way.)
>
> Many thanks!
> Kevin
>
>
>
> bash-4.2$ hg diff
> diff -r 84212a4e67e1 src/cpu/sparc/vm/vm_version_sparc.cpp
> --- a/src/cpu/sparc/vm/vm_version_sparc.cpp     Fri Mar 17 07:52:54 2017 -0700
> +++ b/src/cpu/sparc/vm/vm_version_sparc.cpp     Thu Mar 30 09:27:20 2017 -0700
> @@ -259,49 +259,6 @@
>   // buf is started with ", " or is empty
>   _features_str = strdup(strlen(buf) > 2 ? buf + 2 : buf);
>
> -  // There are three 64-bit SPARC families that do not overlap, e.g.,
> -  // both is_ultra3() and is_sparc64() cannot be true at the same time.
> -  // Within these families, there can be more than one chip, e.g.,
> -  // is_T4() and is_T7() machines are also is_niagara().
> -  if (is_ultra3()) {
> -    assert(_L1_data_cache_line_size == 0, "overlap with Ultra3 family");
> -    // Ref: UltraSPARC III Cu Processor
> -    _L1_data_cache_line_size = 64;
> -  }
> -  if (is_niagara()) {
> -    assert(_L1_data_cache_line_size == 0, "overlap with niagara family");
> -    // All Niagara's are sun4v's, but not all sun4v's are Niagaras, e.g.,
> -    // Fujitsu SPARC64 is sun4v, but we don't want it in this block.
> -    //
> -    // Ref: UltraSPARC T1 Supplement to the UltraSPARC Architecture 2005
> -    // Appendix F.1.3.1 Cacheable Accesses
> -    // -> 16-byte L1 cache line size
> -    //
> -    // Ref: UltraSPARC T2: A Highly-Threaded, Power-Efficient, SPARC SOC
> -    // Section III: SPARC Processor Core
> -    // -> 16-byte L1 cache line size
> -    //
> -    // Ref: Oracle's SPARC T4-1, SPARC T4-2, SPARC T4-4, and SPARC T4-1B Server Architecture
> -    // Section SPARC T4 Processor Cache Architecture
> -    // -> 32-byte L1 cache line size (no longer see that info on this ref)
> -    //
> -    // XXX - still need a T7 reference here
> -    //
> -    if (is_T7()) {  // T7 or newer
> -      _L1_data_cache_line_size = 64;
> -    } else if (is_T4()) {  // T4 or newer (until T7)
> -      _L1_data_cache_line_size = 32;
> -    } else {  // T1 or newer (until T4)
> -      _L1_data_cache_line_size = 16;
> -    }
> -  }
> -  if (is_sparc64()) {
> -    guarantee(_L1_data_cache_line_size == 0, "overlap with SPARC64 family");
> -    // Ref: Fujitsu SPARC64 VII Processor
> -    // Section 4 Cache System
> -    _L1_data_cache_line_size = 64;
> -  }
> -
>   // UseVIS is set to the smallest of what hardware supports and what
>   // the command line requires.  I.e., you cannot set UseVIS to 3 on
>   // older UltraSparc which do not support it.
>
>

Loading...