Skip to content

Commit 35aeb88

Browse files
committed
cleanups & refactorings
1 parent 650e3d6 commit 35aeb88

File tree

9 files changed

+105
-134
lines changed

9 files changed

+105
-134
lines changed

src/hotspot/cpu/x86/vm_version_x86.cpp

Lines changed: 11 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,10 @@ address VM_Version::_cpuinfo_cont_addr_apx = nullptr;
6363
static BufferBlob* stub_blob;
6464
static const int stub_size = 2000;
6565

66-
uint32_t VM_Features::_features_vector_element_shift_count = 6;
67-
uint32_t VM_Features::_features_vector_size = MAX_FEATURE_VEC_SIZE;
68-
VM_Features VM_Version::_features;
69-
VM_Features VM_Version::_cpu_features;
66+
int VM_Version::VM_Features::_features_bitmap_size_in_bytes = sizeof(VM_Version::VM_Features::_features_bitmap);
67+
68+
VM_Version::VM_Features VM_Version::_features;
69+
VM_Version::VM_Features VM_Version::_cpu_features;
7070

7171
extern "C" {
7272
typedef void (*get_cpu_info_stub_t)(void*);
@@ -1101,11 +1101,8 @@ void VM_Version::get_processor_features() {
11011101
cores_per_cpu(), threads_per_core(),
11021102
cpu_family(), _model, _stepping, os::cpu_microcode_revision());
11031103
assert(cpu_info_size > 0, "not enough temporary space allocated");
1104-
size_t buf_iter = cpu_info_size;
1105-
for (uint64_t i = 0; i < VM_Features::features_vector_size(); i++) {
1106-
insert_features_names(features_vector_elem(i), buf + buf_iter, sizeof(buf) - buf_iter, _features_names, 64 * i);
1107-
buf_iter = strlen(buf);
1108-
}
1104+
1105+
insert_features_names(_features, buf + cpu_info_size, sizeof(buf) - cpu_info_size);
11091106

11101107
_cpu_info_string = os::strdup(buf);
11111108

@@ -2107,7 +2104,6 @@ static bool _vm_version_initialized = false;
21072104

21082105
void VM_Version::initialize() {
21092106
ResourceMark rm;
2110-
assert(VM_Features::is_within_feature_vector_bounds(MAX_CPU_FEATURES), "Feature out of vector bounds");
21112107

21122108
// Making this stub must be FIRST use of assembler
21132109
stub_blob = BufferBlob::create("VM_Version stub", stub_size);
@@ -2884,7 +2880,7 @@ int64_t VM_Version::maximum_qualified_cpu_frequency(void) {
28842880
return _max_qualified_cpu_frequency;
28852881
}
28862882

2887-
VM_Features VM_Version::CpuidInfo::feature_flags() const {
2883+
VM_Version::VM_Features VM_Version::CpuidInfo::feature_flags() const {
28882884
VM_Features vm_features;
28892885
if (std_cpuid1_edx.bits.cmpxchg8 != 0)
28902886
vm_features.set_feature(CPU_CX8);
@@ -3280,47 +3276,13 @@ bool VM_Version::is_intrinsic_supported(vmIntrinsicID id) {
32803276
return true;
32813277
}
32823278

3283-
void VM_Features::set_feature(uint32_t feature) {
3284-
uint32_t index = feature >> _features_vector_element_shift_count;
3285-
uint32_t index_mask = (1 << _features_vector_element_shift_count) - 1;
3286-
assert(index < _features_vector_size, "Features array index out of bounds");
3287-
_features_vector[index] |= (1ULL << (feature & index_mask));
3288-
}
3289-
3290-
void VM_Features::clear_feature(uint32_t feature) {
3291-
uint32_t index = feature >> _features_vector_element_shift_count;
3292-
uint32_t index_mask = (1 << _features_vector_element_shift_count) - 1;
3293-
assert(index < _features_vector_size, "Features array index out of bounds");
3294-
_features_vector[index] &= ~(1ULL << (feature & index_mask));
3295-
}
3296-
3297-
void VM_Features::clear_features() {
3298-
for (uint32_t i = 0; i < _features_vector_size; i++) {
3299-
_features_vector[i] = 0;
3300-
}
3301-
}
3302-
3303-
bool VM_Features::supports_feature(uint32_t feature) {
3304-
uint32_t index = feature >> _features_vector_element_shift_count;
3305-
uint32_t index_mask = (1 << _features_vector_element_shift_count) - 1;
3306-
assert(index < _features_vector_size, "Features array index out of bounds");
3307-
return (_features_vector[index] & (1ULL << (feature & index_mask))) != 0;
3308-
}
3309-
3310-
bool VM_Features::is_within_feature_vector_bounds(uint32_t num_features) {
3311-
return _features_vector_size >= ((num_features >> _features_vector_element_shift_count) + 1);
3312-
}
3313-
3314-
void VM_Version::insert_features_names(uint64_t features, char* buf, size_t buflen, const char* features_names[],
3315-
uint features_names_index) {
3316-
while (features != 0) {
3317-
if (features & 1) {
3318-
int res = jio_snprintf(buf, buflen, ", %s", features_names[features_names_index]);
3279+
void VM_Version::insert_features_names(VM_Version::VM_Features features, char* buf, size_t buflen) {
3280+
for (int i = 0; i < MAX_CPU_FEATURES; i++) {
3281+
if (features.supports_feature((VM_Version::Feature_Flag)i)) {
3282+
int res = jio_snprintf(buf, buflen, ", %s", _features_names[i]);
33193283
assert(res > 0, "not enough temporary space allocated");
33203284
buf += res;
33213285
buflen -= res;
33223286
}
3323-
features >>= 1;
3324-
++features_names_index;
33253287
}
33263288
}

src/hotspot/cpu/x86/vm_version_x86.hpp

Lines changed: 61 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -30,35 +30,6 @@
3030
#include "utilities/macros.hpp"
3131
#include "utilities/sizes.hpp"
3232

33-
#define MAX_FEATURE_VEC_SIZE 4
34-
35-
class VM_Features {
36-
public:
37-
using FeatureVector = uint64_t [MAX_FEATURE_VEC_SIZE];
38-
39-
// Feature vector bitmap currently only used by x86 backend.
40-
FeatureVector _features_vector;
41-
42-
// log2 of feature vector element size in bits, used by JVMCI to check enabled feature bits.
43-
// Refer HotSpotJVMCIBackendFactory::convertFeaturesVector.
44-
static uint32_t _features_vector_element_shift_count;
45-
46-
// Size of feature vector bitmap.
47-
static uint32_t _features_vector_size;
48-
49-
VM_Features() {
50-
clear_features();
51-
}
52-
53-
void set_feature(uint32_t feature);
54-
void clear_feature(uint32_t feature);
55-
bool supports_feature(uint32_t feature);
56-
void clear_features();
57-
58-
static bool is_within_feature_vector_bounds(uint32_t num_features);
59-
static uint32_t features_vector_size() { return _features_vector_size;}
60-
};
61-
6233
class VM_Version : public Abstract_VM_Version {
6334
friend class VMStructs;
6435
friend class JVMCIVMStructs;
@@ -393,7 +364,7 @@ class VM_Version : public Abstract_VM_Version {
393364
* test/lib-test/jdk/test/whitebox/CPUInfoTest.java
394365
* src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/amd64/AMD64.java
395366
*/
396-
enum Feature_Flag : uint32_t {
367+
enum Feature_Flag {
397368
#define CPU_FEATURE_FLAGS(decl) \
398369
decl(CX8, "cx8", 0) /* next bits are from cpuid 1 (EDX) */ \
399370
decl(CMOV, "cmov", 1) \
@@ -479,22 +450,73 @@ class VM_Version : public Abstract_VM_Version {
479450
MAX_CPU_FEATURES
480451
};
481452

482-
static const char* _features_names[];
453+
class VM_Features {
454+
friend class VMStructs;
455+
friend class JVMCIVMStructs;
456+
457+
private:
458+
uint64_t _features_bitmap[2]; // tracks up to 128 features
459+
460+
STATIC_ASSERT(sizeof(_features_bitmap) * BitsPerByte > MAX_CPU_FEATURES);
461+
462+
// Number of 8-byte elements in _bitmap.
463+
constexpr static int features_bitmap_element_count() {
464+
return sizeof(_features_bitmap) / sizeof(uint64_t);
465+
}
466+
467+
constexpr static int features_bitmap_element_shift_count() {
468+
return LogBitsPerLong;
469+
}
470+
471+
constexpr static uint64_t features_bitmap_element_mask() {
472+
return (1ULL << features_bitmap_element_shift_count()) - 1;
473+
}
474+
475+
static int index(Feature_Flag feature) {
476+
int idx = feature >> features_bitmap_element_shift_count();
477+
assert(idx < features_bitmap_element_count(), "Features array index out of bounds");
478+
return idx;
479+
}
480+
481+
static uint64_t bit_mask(Feature_Flag feature) {
482+
return (1ULL << (feature & features_bitmap_element_mask()));
483+
}
484+
485+
static int _features_bitmap_size_in_bytes; // for JVMCI purposes
486+
public:
487+
VM_Features() {
488+
for (int i = 0; i < features_bitmap_element_count(); i++) {
489+
_features_bitmap[i] = 0;
490+
}
491+
}
492+
493+
void set_feature(Feature_Flag feature) {
494+
int idx = index(feature);
495+
_features_bitmap[idx] |= bit_mask(feature);
496+
}
497+
498+
void clear_feature(VM_Version::Feature_Flag feature) {
499+
int idx = index(feature);
500+
_features_bitmap[idx] &= ~bit_mask(feature);
501+
}
502+
503+
bool supports_feature(VM_Version::Feature_Flag feature) {
504+
int idx = index(feature);
505+
return (_features_bitmap[idx] & bit_mask(feature)) != 0;
506+
}
507+
};
483508

484509
// CPU feature flags vector, can be affected by VM settings.
485510
static VM_Features _features;
486511

487512
// Original CPU feature flags vector, not affected by VM settings.
488513
static VM_Features _cpu_features;
489514

490-
static void clear_cpu_features() {
491-
_features.clear_features();
492-
_cpu_features.clear_features();
493-
}
515+
static const char* _features_names[];
494516

495-
static uint64_t features_vector_elem(uint32_t elem) {
496-
assert(elem < VM_Features::_features_vector_size, "");
497-
return _features._features_vector[elem];
517+
static void clear_cpu_features() {
518+
_features = VM_Features();
519+
_cpu_features = VM_Features();
498520
}
499521

500522
enum Extended_Family {
@@ -899,7 +921,7 @@ class VM_Version : public Abstract_VM_Version {
899921

900922
static bool is_intel_tsc_synched_at_init();
901923

902-
static void insert_features_names(uint64_t features, char* buf, size_t buflen, const char* features_names[], uint features_names_index = 0);
924+
static void insert_features_names(VM_Version::VM_Features features, char* buf, size_t buflen);
903925

904926
// This checks if the JVM is potentially affected by an erratum on Intel CPUs (SKX102)
905927
// that causes unpredictable behaviour when jcc crosses 64 byte boundaries. Its microcode

src/hotspot/share/jvmci/jvmciCompilerToVMInit.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ jobjectArray readConfiguration0(JNIEnv *env, JVMCI_TRAPS) {
449449
strcmp(vmField.typeString, "intptr_t") == 0 ||
450450
strcmp(vmField.typeString, "uintptr_t") == 0 ||
451451
strcmp(vmField.typeString, "OopHandle") == 0 ||
452-
strcmp(vmField.typeString, "VM_Features") == 0 ||
452+
strcmp(vmField.typeString, "VM_Version::VM_Features") == 0 ||
453453
strcmp(vmField.typeString, "size_t") == 0 ||
454454
// All foo* types are addresses.
455455
vmField.typeString[strlen(vmField.typeString) - 1] == '*') {

src/hotspot/share/jvmci/vmStructs_jvmci.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -975,12 +975,11 @@
975975

976976
#define VM_STRUCTS_CPU(nonstatic_field, static_field, unchecked_nonstatic_field, volatile_nonstatic_field, nonproduct_nonstatic_field) \
977977
volatile_nonstatic_field(JavaFrameAnchor, _last_Java_fp, intptr_t*) \
978-
static_field(VM_Version, _features, VM_Features) \
979-
\
980-
nonstatic_field(VM_Features, _features_vector, VM_Features::FeatureVector) \
981-
static_field(VM_Features, _features_vector_size, uint32_t) \
982-
static_field(VM_Features, _features_vector_element_shift_count, uint32_t) \
983-
static_field(VM_Version, _has_intel_jcc_erratum, bool)
978+
static_field(VM_Version, _features, VM_Version::VM_Features) \
979+
\
980+
nonstatic_field(VM_Version::VM_Features, _features_bitmap[0], uint64_t) \
981+
static_field(VM_Version::VM_Features, _features_bitmap_size_in_bytes, int) \
982+
static_field(VM_Version, _has_intel_jcc_erratum, bool)
984983

985984
#define VM_INT_CONSTANTS_CPU(declare_constant, declare_preprocessor_constant) \
986985
LP64_ONLY(declare_constant(frame::arg_reg_save_area_bytes)) \

src/hotspot/share/runtime/abstract_vm_version.hpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,6 @@ class Abstract_VM_Version: AllStatic {
129129
static const char* jdk_debug_level();
130130
static const char* printable_jdk_debug_level();
131131

132-
static uint64_t features() { return _features; }
133-
134132
static const char* features_string() { return _features_string; }
135133

136134
static const char* cpu_info_string() { return _cpu_info_string; }

src/hotspot/share/runtime/vmStructs.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -708,10 +708,9 @@
708708
static_field(Abstract_VM_Version, _vm_security_version, int) \
709709
static_field(Abstract_VM_Version, _vm_build_number, int) \
710710
\
711-
NOT_ZERO(X86_ONLY(static_field(VM_Version, _features, VM_Features))) \
712-
NOT_ZERO(X86_ONLY(nonstatic_field(VM_Features, _features_vector, VM_Features::FeatureVector))) \
713-
NOT_ZERO(X86_ONLY(static_field(VM_Features, _features_vector_size, uint32_t))) \
714-
NOT_ZERO(X86_ONLY(static_field(VM_Features, _features_vector_element_shift_count, uint32_t))) \
711+
NOT_ZERO(X86_ONLY( static_field(VM_Version, _features, VM_Version::VM_Features))) \
712+
NOT_ZERO(X86_ONLY(nonstatic_field(VM_Version::VM_Features, _features_bitmap[0], uint64_t))) \
713+
NOT_ZERO(X86_ONLY( static_field(VM_Version::VM_Features, _features_bitmap_size_in_bytes, int))) \
715714
\
716715
/*************************/ \
717716
/* JVMTI */ \
@@ -1169,9 +1168,8 @@
11691168
/********************/ \
11701169
\
11711170
declare_toplevel_type(Abstract_VM_Version) \
1172-
NOT_ZERO(X86_ONLY(declare_toplevel_type(VM_Features))) \
11731171
NOT_ZERO(declare_toplevel_type(VM_Version)) \
1174-
NOT_ZERO(X86_ONLY(declare_toplevel_type(VM_Features::FeatureVector))) \
1172+
NOT_ZERO(X86_ONLY(declare_toplevel_type(VM_Version::VM_Features))) \
11751173
\
11761174
/*************/ \
11771175
/* Arguments */ \

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotJVMCIBackendFactory.java

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -83,28 +83,24 @@ static <CPUFeatureType extends Enum<CPUFeatureType>> EnumSet<CPUFeatureType> con
8383
}
8484

8585
/**
86-
* Converts a dynamically sized CPU features vector into enum constants.
86+
* Converts CPU features bit map into enum constants.
8787
*
8888
* @param <CPUFeatureType> CPU feature enum type
8989
* @param enumType the class of {@code CPUFeatureType}
9090
* @param constants VM constants. Each entry whose key starts with {@code "VM_Version::CPU_"}
9191
* specifies a CPU feature and its value is a mask for a bit in {@code features}
92-
* @param features_pointer pointer to {@code _vm_target_features} field of {@code Abstract_VM_Version}
93-
* @param features_vector_offset offset of feature_vector field in {@code VM_Features}
94-
* @param features_vector_size size of feature vector
95-
* @param features_element_shift_count log2 of dynamic feature bit vector lanesize in bits.
92+
* @param featuresBitMapAddress pointer to {@code VM_Features::_features_bitmap} field of {@code VM_Version::_features}
93+
* @param featuresBitMapSize size of feature bit map in bytes
9694
* @param renaming maps from VM feature names to enum constant names where the two differ
9795
* @throws IllegalArgumentException if any VM CPU feature constant cannot be converted to an
9896
* enum value
9997
* @return the set of converted values
10098
*/
101-
static <CPUFeatureType extends Enum<CPUFeatureType>> EnumSet<CPUFeatureType> convertFeaturesVector(
99+
static <CPUFeatureType extends Enum<CPUFeatureType>> EnumSet<CPUFeatureType> convertFeatures(
102100
Class<CPUFeatureType> enumType,
103101
Map<String, Long> constants,
104-
long features_pointer,
105-
long features_vector_offset,
106-
long features_vector_size,
107-
long features_element_shift_count,
102+
long featuresBitMapAddress,
103+
long featuresBitMapSize,
108104
Map<String, String> renaming) {
109105
EnumSet<CPUFeatureType> outFeatures = EnumSet.noneOf(enumType);
110106
List<String> missing = new ArrayList<>();
@@ -115,19 +111,18 @@ static <CPUFeatureType extends Enum<CPUFeatureType>> EnumSet<CPUFeatureType> con
115111
if (key.startsWith("VM_Version::CPU_")) {
116112
String name = key.substring("VM_Version::CPU_".length());
117113
try {
114+
final long featuresElementShiftCount = 6; // log (# of bits per long)
115+
final long featuresElementMask = (1L << featuresElementShiftCount) - 1;
116+
118117
CPUFeatureType feature = Enum.valueOf(enumType, renaming.getOrDefault(name, name));
119-
long features_vector_index = bitIndex >>> features_element_shift_count;
120-
assert features_vector_index < features_vector_size;
121118

122-
long features_element_bitsize = (1L << features_element_shift_count);
123-
assert (features_element_bitsize & (features_element_bitsize - 1)) == 0;
119+
long featureIndex = bitIndex >>> featuresElementShiftCount;
120+
long featureBitMask = 1L << (bitIndex & featuresElementMask);
121+
assert featureIndex < featuresBitMapSize;
124122

125-
long features_element_size = features_element_bitsize / Byte.SIZE;
126-
long features = UNSAFE.getLong(features_pointer + features_vector_offset +
127-
features_vector_index * features_element_size);
123+
long featuresElement = UNSAFE.getLong(featuresBitMapAddress + featureIndex * Long.BYTES);
128124

129-
long effective_bitMask = 1L << (bitIndex & (features_element_bitsize - 1));
130-
if ((features & effective_bitMask) != 0) {
125+
if ((featuresElement & featureBitMask) != 0) {
131126
outFeatures.add(feature);
132127
}
133128
} catch (IllegalArgumentException iae) {

src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/amd64/AMD64HotSpotJVMCIBackendFactory.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ private static EnumSet<CPUFeature> computeFeatures(AMD64HotSpotVMConfig config)
5050
Map<String, Long> constants = config.getStore().getConstants();
5151
Map<String, String> renaming = Map.of("3DNOW_PREFETCH", "AMD_3DNOW_PREFETCH");
5252
assert config.useSSE >= 2 : "minimum config for x64";
53-
EnumSet<CPUFeature> features = HotSpotJVMCIBackendFactory.convertFeaturesVector(CPUFeature.class, constants,
54-
config.vmVersionFeatures,
55-
config.vmFeaturesFeaturesVecOffset,
56-
config.vmFeaturesFeaturesVecSize,
57-
config.vmFeaturesFeaturesElemShiftCnt,
58-
renaming);
53+
long featuresBitMapAddress = config.vmVersionFeatures + config.vmFeaturesFeaturesOffset;
54+
EnumSet<CPUFeature> features = HotSpotJVMCIBackendFactory.convertFeatures(CPUFeature.class,
55+
constants,
56+
featuresBitMapAddress,
57+
config.vmFeaturesFeaturesSize,
58+
renaming);
5959
features.add(AMD64.CPUFeature.SSE);
6060
features.add(AMD64.CPUFeature.SSE2);
6161
return features;

0 commit comments

Comments
 (0)