From 1f5a033421bbcf803169c5c5f93314fd22b5a4a5 Mon Sep 17 00:00:00 2001
From: Stefan Karlsson <stefank@openjdk.org>
Date: Fri, 25 Sep 2020 10:29:26 +0000
Subject: [PATCH] 8253555: Make ByteSize and WordSize typed scoped enums
Reviewed-by: kbarrett, tschatzl
---
src/hotspot/cpu/aarch64/assembler_aarch64.hpp | 4 +-
src/hotspot/cpu/arm/assembler_arm.hpp | 13 +--
src/hotspot/cpu/arm/assembler_arm_32.hpp | 8 +-
src/hotspot/cpu/ppc/assembler_ppc.hpp | 4 +-
src/hotspot/cpu/ppc/assembler_ppc.inline.hpp | 4 +-
src/hotspot/cpu/s390/assembler_s390.hpp | 11 +-
src/hotspot/cpu/x86/assembler_x86.hpp | 58 +++-------
src/hotspot/share/utilities/sizes.hpp | 108 ++----------------
test/hotspot/gtest/utilities/test_sizes.cpp | 41 +++++++
9 files changed, 74 insertions(+), 177 deletions(-)
create mode 100644 test/hotspot/gtest/utilities/test_sizes.cpp
diff --git a/src/hotspot/cpu/aarch64/assembler_aarch64.hpp b/src/hotspot/cpu/aarch64/assembler_aarch64.hpp
index 19a3ced1435..717be156e5c 100644
--- a/src/hotspot/cpu/aarch64/assembler_aarch64.hpp
+++ b/src/hotspot/cpu/aarch64/assembler_aarch64.hpp
@@ -401,10 +401,8 @@ class Address {
: _base(r), _index(noreg), _offset(o), _mode(base_plus_offset), _target(0) { }
Address(Register r, unsigned long long o)
: _base(r), _index(noreg), _offset(o), _mode(base_plus_offset), _target(0) { }
-#ifdef ASSERT
Address(Register r, ByteSize disp)
- : _base(r), _index(noreg), _offset(in_bytes(disp)), _mode(base_plus_offset), _target(0) { }
-#endif
+ : Address(r, in_bytes(disp)) { }
Address(Register r, Register r1, extend ext = lsl())
: _base(r), _index(r1), _offset(0), _mode(base_plus_offset_reg),
_ext(ext), _target(0) { }
diff --git a/src/hotspot/cpu/arm/assembler_arm.hpp b/src/hotspot/cpu/arm/assembler_arm.hpp
index 8beaa284a79..1de4b0da774 100644
--- a/src/hotspot/cpu/arm/assembler_arm.hpp
+++ b/src/hotspot/cpu/arm/assembler_arm.hpp
@@ -89,17 +89,8 @@ class Address {
_offset_op = add_offset;
}
-#ifdef ASSERT
- Address(Register rn, ByteSize offset, AsmOffset mode = basic_offset) {
- _base = rn;
- _index = noreg;
- _disp = in_bytes(offset);
- _mode = mode;
- _shift_imm = 0;
- _shift = lsl;
- _offset_op = add_offset;
- }
-#endif
+ Address(Register rn, ByteSize offset, AsmOffset mode = basic_offset) :
+ Address(rn, in_bytes(offset), mode) {}
Address(Register rn, Register rm, AsmShift shift = lsl,
int shift_imm = 0, AsmOffset mode = basic_offset,
diff --git a/src/hotspot/cpu/arm/assembler_arm_32.hpp b/src/hotspot/cpu/arm/assembler_arm_32.hpp
index 44997ddcbce..dd04ad1ab3a 100644
--- a/src/hotspot/cpu/arm/assembler_arm_32.hpp
+++ b/src/hotspot/cpu/arm/assembler_arm_32.hpp
@@ -55,12 +55,8 @@ class AsmOperand {
encode(imm_8);
}
-#ifdef ASSERT
- AsmOperand(ByteSize bytesize_8) {
- const int imm_8 = in_bytes(bytesize_8);
- encode(imm_8);
- }
-#endif // ASSERT
+ AsmOperand(ByteSize bytesize_8) :
+ AsmOperand(in_bytes(bytesize_8)) {}
AsmOperand(Register rm, AsmShift shift, int shift_imm) {
encode(rm,shift,shift_imm);
diff --git a/src/hotspot/cpu/ppc/assembler_ppc.hpp b/src/hotspot/cpu/ppc/assembler_ppc.hpp
index 37a46a26b5a..05ab385fb4f 100644
--- a/src/hotspot/cpu/ppc/assembler_ppc.hpp
+++ b/src/hotspot/cpu/ppc/assembler_ppc.hpp
@@ -1639,7 +1639,7 @@ class Assembler : public AbstractAssembler {
// For convenience. Load pointer into d from b+s1.
inline void ld_ptr(Register d, int b, Register s1);
- DEBUG_ONLY(inline void ld_ptr(Register d, ByteSize b, Register s1);)
+ inline void ld_ptr(Register d, ByteSize b, Register s1);
// PPC 1, section 3.3.3 Fixed-Point Store Instructions
inline void stwx( Register d, Register s1, Register s2);
@@ -1663,7 +1663,7 @@ class Assembler : public AbstractAssembler {
inline void stdbrx( Register d, Register s1, Register s2);
inline void st_ptr(Register d, int si16, Register s1);
- DEBUG_ONLY(inline void st_ptr(Register d, ByteSize b, Register s1);)
+ inline void st_ptr(Register d, ByteSize b, Register s1);
// PPC 1, section 3.3.13 Move To/From System Register Instructions
inline void mtlr( Register s1);
diff --git a/src/hotspot/cpu/ppc/assembler_ppc.inline.hpp b/src/hotspot/cpu/ppc/assembler_ppc.inline.hpp
index 4f50b570f68..a351674b5ba 100644
--- a/src/hotspot/cpu/ppc/assembler_ppc.inline.hpp
+++ b/src/hotspot/cpu/ppc/assembler_ppc.inline.hpp
@@ -342,7 +342,7 @@ inline void Assembler::ldu( Register d, int si16, Register s1) { assert(d !=
inline void Assembler::ldbrx( Register d, Register s1, Register s2) { emit_int32(LDBRX_OPCODE | rt(d) | ra0mem(s1) | rb(s2));}
inline void Assembler::ld_ptr(Register d, int b, Register s1) { ld(d, b, s1); }
-DEBUG_ONLY(inline void Assembler::ld_ptr(Register d, ByteSize b, Register s1) { ld(d, in_bytes(b), s1); })
+inline void Assembler::ld_ptr(Register d, ByteSize b, Register s1) { ld(d, in_bytes(b), s1); }
// PPC 1, section 3.3.3 Fixed-Point Store Instructions
inline void Assembler::stwx( Register d, Register s1, Register s2) { emit_int32(STWX_OPCODE | rs(d) | ra0mem(s1) | rb(s2));}
@@ -366,7 +366,7 @@ inline void Assembler::stdux(Register s, Register a, Register b) { emit_int32(
inline void Assembler::stdbrx( Register d, Register s1, Register s2) { emit_int32(STDBRX_OPCODE | rs(d) | ra0mem(s1) | rb(s2));}
inline void Assembler::st_ptr(Register d, int b, Register s1) { std(d, b, s1); }
-DEBUG_ONLY(inline void Assembler::st_ptr(Register d, ByteSize b, Register s1) { std(d, in_bytes(b), s1); })
+inline void Assembler::st_ptr(Register d, ByteSize b, Register s1) { std(d, in_bytes(b), s1); }
// PPC 1, section 3.3.13 Move To/From System Register Instructions
inline void Assembler::mtlr( Register s1) { emit_int32(MTLR_OPCODE | rs(s1)); }
diff --git a/src/hotspot/cpu/s390/assembler_s390.hpp b/src/hotspot/cpu/s390/assembler_s390.hpp
index cd1e7e2c5f2..96e494d52e7 100644
--- a/src/hotspot/cpu/s390/assembler_s390.hpp
+++ b/src/hotspot/cpu/s390/assembler_s390.hpp
@@ -206,18 +206,11 @@ class Address {
if (roc.is_constant()) _disp += roc.as_constant(); else _index = roc.as_register();
}
-#ifdef ASSERT
- // ByteSize is only a class when ASSERT is defined, otherwise it's an int.
Address(Register base, ByteSize disp) :
- _base(base),
- _index(noreg),
- _disp(in_bytes(disp)) {}
+ Address(base, in_bytes(disp)) {}
Address(Register base, Register index, ByteSize disp) :
- _base(base),
- _index(index),
- _disp(in_bytes(disp)) {}
-#endif
+ Address(base, index, in_bytes(disp)) {}
// Aborts if disp is a register and base and index are set already.
Address plus_disp(RegisterOrConstant disp) const {
diff --git a/src/hotspot/cpu/x86/assembler_x86.hpp b/src/hotspot/cpu/x86/assembler_x86.hpp
index bbcfb7ec64e..283285dc347 100644
--- a/src/hotspot/cpu/x86/assembler_x86.hpp
+++ b/src/hotspot/cpu/x86/assembler_x86.hpp
@@ -256,6 +256,19 @@ class Address {
"inconsistent address");
}
+ // The following overloads are used in connection with the
+ // ByteSize type (see sizes.hpp). They simplify the use of
+ // ByteSize'd arguments in assembly code.
+
+ Address(Register base, ByteSize disp)
+ : Address(base, in_bytes(disp)) {}
+
+ Address(Register base, Register index, ScaleFactor scale, ByteSize disp)
+ : Address(base, index, scale, in_bytes(disp)) {}
+
+ Address(Register base, RegisterOrConstant index, ScaleFactor scale, ByteSize disp)
+ : Address(base, index, scale, in_bytes(disp)) {}
+
Address plus_disp(int disp) const {
Address a = (*this);
a._disp += disp;
@@ -276,51 +289,6 @@ class Address {
return _base == a._base && _disp == a._disp && _index == a._index && _scale == a._scale;
}
- // The following two overloads are used in connection with the
- // ByteSize type (see sizes.hpp). They simplify the use of
- // ByteSize'd arguments in assembly code. Note that their equivalent
- // for the optimized build are the member functions with int disp
- // argument since ByteSize is mapped to an int type in that case.
- //
- // Note: DO NOT introduce similar overloaded functions for WordSize
- // arguments as in the optimized mode, both ByteSize and WordSize
- // are mapped to the same type and thus the compiler cannot make a
- // distinction anymore (=> compiler errors).
-
-#ifdef ASSERT
- Address(Register base, ByteSize disp)
- : _base(base),
- _index(noreg),
- _xmmindex(xnoreg),
- _scale(no_scale),
- _disp(in_bytes(disp)),
- _isxmmindex(false){
- }
-
- Address(Register base, Register index, ScaleFactor scale, ByteSize disp)
- : _base(base),
- _index(index),
- _xmmindex(xnoreg),
- _scale(scale),
- _disp(in_bytes(disp)),
- _isxmmindex(false){
- assert(!index->is_valid() == (scale == Address::no_scale),
- "inconsistent address");
- }
- Address(Register base, RegisterOrConstant index, ScaleFactor scale, ByteSize disp)
- : _base (base),
- _index(index.register_or_noreg()),
- _xmmindex(xnoreg),
- _scale(scale),
- _disp (in_bytes(disp) + (index.constant_or_zero() * scale_size(scale))),
- _isxmmindex(false) {
- if (!index.is_register()) scale = Address::no_scale;
- assert(!_index->is_valid() == (scale == Address::no_scale),
- "inconsistent address");
- }
-
-#endif // ASSERT
-
// accessors
bool uses(Register reg) const { return _base == reg || _index == reg; }
Register base() const { return _base; }
diff --git a/src/hotspot/share/utilities/sizes.hpp b/src/hotspot/share/utilities/sizes.hpp
index 0944c8280b2..97e28cd6082 100644
--- a/src/hotspot/share/utilities/sizes.hpp
+++ b/src/hotspot/share/utilities/sizes.hpp
@@ -32,22 +32,6 @@
// WordSize is used for sizes measured in machine words (i.e., 32bit or 64bit words
// depending on platform).
//
-// The classes are defined with friend functions operating on them instead of member
-// functions so that they (the classes) can be re-#define'd to int types in optimized
-// mode. This allows full type checking and maximum safety in debug mode, and full
-// optimizations (constant folding) and zero overhead (time and space wise) in the
-// optimized build (some compilers do not optimize one-element value classes but
-// instead create an object in memory - thus the overhead may be significant).
-//
-// Note: 1) DO NOT add new overloaded friend functions that do not have a unique function
-// function name but require signature types for resolution. This will not work
-// in optimized mode as both, ByteSize and WordSize are mapped to the same type
-// and thus the distinction would not be possible anymore (=> compiler errors).
-//
-// 2) DO NOT add non-static member functions as they cannot be mapped so something
-// compilable in the optimized build. Static member functions could be added
-// but require a corresponding class definition in the optimized build.
-//
// These classes should help doing a transition from (currently) word-size based offsets
// to byte-size based offsets in the VM (this will be important if we desire to pack
// objects more densely in the VM for 64bit machines). Such a transition should proceed
@@ -56,93 +40,19 @@
// a) first transition the whole VM into a form where all sizes are strongly typed
// b) change all WordSize's to ByteSize's where desired and fix the compilation errors
+enum class WordSize : int {};
-#ifdef ASSERT
+constexpr WordSize in_WordSize(int size) { return static_cast<WordSize>(size); }
+constexpr int in_words(WordSize x) { return static_cast<int>(x); }
-class ByteSize {
- private:
- int _size;
+enum class ByteSize : int {};
- // Note: This constructor must be private to avoid implicit conversions!
- ByteSize(int size) { _size = size; }
-
- public:
- // constructors
- inline friend ByteSize in_ByteSize(int size);
-
- // accessors
- inline friend int in_bytes(ByteSize x);
-
- // operators
- friend ByteSize operator + (ByteSize x, ByteSize y) { return ByteSize(in_bytes(x) + in_bytes(y)); }
- friend ByteSize operator - (ByteSize x, ByteSize y) { return ByteSize(in_bytes(x) - in_bytes(y)); }
- friend ByteSize operator * (ByteSize x, int y) { return ByteSize(in_bytes(x) * y ); }
-
- // comparison
- friend bool operator == (ByteSize x, ByteSize y) { return in_bytes(x) == in_bytes(y); }
- friend bool operator != (ByteSize x, ByteSize y) { return in_bytes(x) != in_bytes(y); }
- friend bool operator < (ByteSize x, ByteSize y) { return in_bytes(x) < in_bytes(y); }
- friend bool operator <= (ByteSize x, ByteSize y) { return in_bytes(x) <= in_bytes(y); }
- friend bool operator > (ByteSize x, ByteSize y) { return in_bytes(x) > in_bytes(y); }
- friend bool operator >= (ByteSize x, ByteSize y) { return in_bytes(x) >= in_bytes(y); }
-};
-
-inline ByteSize in_ByteSize(int size) { return ByteSize(size); }
-inline int in_bytes(ByteSize x) { return x._size; }
-
-
-class WordSize {
- private:
- int _size;
-
- // Note: This constructor must be private to avoid implicit conversions!
- WordSize(int size) { _size = size; }
-
- public:
- // constructors
- inline friend WordSize in_WordSize(int size);
-
- // accessors
- inline friend int in_words(WordSize x);
-
- // operators
- friend WordSize operator + (WordSize x, WordSize y) { return WordSize(in_words(x) + in_words(y)); }
- friend WordSize operator - (WordSize x, WordSize y) { return WordSize(in_words(x) - in_words(y)); }
- friend WordSize operator * (WordSize x, int y) { return WordSize(in_words(x) * y ); }
-
- // comparison
- friend bool operator == (WordSize x, WordSize y) { return in_words(x) == in_words(y); }
- friend bool operator != (WordSize x, WordSize y) { return in_words(x) != in_words(y); }
- friend bool operator < (WordSize x, WordSize y) { return in_words(x) < in_words(y); }
- friend bool operator <= (WordSize x, WordSize y) { return in_words(x) <= in_words(y); }
- friend bool operator > (WordSize x, WordSize y) { return in_words(x) > in_words(y); }
- friend bool operator >= (WordSize x, WordSize y) { return in_words(x) >= in_words(y); }
-};
-
-inline WordSize in_WordSize(int size) { return WordSize(size); }
-inline int in_words(WordSize x) { return x._size; }
-
-
-#else // ASSERT
-
-// The following definitions must match the corresponding friend declarations
-// in the Byte/WordSize classes if they are typedef'ed to be int. This will
-// be the case in optimized mode to ensure zero overhead for these types.
-//
-// Note: If a compiler does not inline these function calls away, one may
-// want to use #define's to make sure full optimization (constant
-// folding in particular) is possible.
-
-typedef int ByteSize;
-inline ByteSize in_ByteSize(int size) { return size; }
-inline int in_bytes (ByteSize x) { return x; }
-
-typedef int WordSize;
-inline WordSize in_WordSize(int size) { return size; }
-inline int in_words (WordSize x) { return x; }
-
-#endif // ASSERT
+constexpr ByteSize in_ByteSize(int size) { return static_cast<ByteSize>(size); }
+constexpr int in_bytes(ByteSize x) { return static_cast<int>(x); }
+constexpr ByteSize operator + (ByteSize x, ByteSize y) { return in_ByteSize(in_bytes(x) + in_bytes(y)); }
+constexpr ByteSize operator - (ByteSize x, ByteSize y) { return in_ByteSize(in_bytes(x) - in_bytes(y)); }
+constexpr ByteSize operator * (ByteSize x, int y) { return in_ByteSize(in_bytes(x) * y ); }
// Use the following #define to get C++ field member offsets
diff --git a/test/hotspot/gtest/utilities/test_sizes.cpp b/test/hotspot/gtest/utilities/test_sizes.cpp
new file mode 100644
index 00000000000..9458124ee2b
--- /dev/null
+++ b/test/hotspot/gtest/utilities/test_sizes.cpp
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+#include "precompiled.hpp"
+#include "utilities/sizes.hpp"
+#include "unittest.hpp"
+
+TEST(ByteSize, constructors) {
+ EXPECT_EQ(in_bytes(in_ByteSize(10)), 10);
+}
+
+TEST(ByteSize, operators) {
+ ByteSize s = in_ByteSize(7);
+ ASSERT_EQ(in_bytes(s + in_ByteSize(3)), 10);
+ ASSERT_EQ(in_bytes(s - in_ByteSize(3)), 4);
+ ASSERT_EQ(in_bytes(s * 3), 21);
+}
+
+TEST(WordSize, constructors) {
+ EXPECT_EQ(in_words(in_WordSize(10)), 10);
+}