# HG changeset patch # User eosterlund # Date 1573122509 0 # Node ID 5d462d4b7a8bcc9f0db647398955a70d4e6a119f # Parent f79a8217d4c9eab38583641ad61a7205c65b8ac2 8233073: Make BitMap accessors more memory ordering friendly Reviewed-by: kbarrett, pliden diff -r f79a8217d4c9 -r 5d462d4b7a8b src/hotspot/share/c1/c1_Instruction.cpp --- a/src/hotspot/share/c1/c1_Instruction.cpp Tue Oct 22 11:55:58 2019 +0200 +++ b/src/hotspot/share/c1/c1_Instruction.cpp Thu Nov 07 10:28:29 2019 +0000 @@ -29,6 +29,7 @@ #include "c1/c1_ValueStack.hpp" #include "ci/ciObjArrayKlass.hpp" #include "ci/ciTypeArrayKlass.hpp" +#include "utilities/bitMap.inline.hpp" // Implementation of Instruction diff -r f79a8217d4c9 -r 5d462d4b7a8b src/hotspot/share/opto/graphKit.cpp --- a/src/hotspot/share/opto/graphKit.cpp Tue Oct 22 11:55:58 2019 +0200 +++ b/src/hotspot/share/opto/graphKit.cpp Thu Nov 07 10:28:29 2019 +0000 @@ -43,6 +43,7 @@ #include "opto/runtime.hpp" #include "runtime/deoptimization.hpp" #include "runtime/sharedRuntime.hpp" +#include "utilities/bitMap.inline.hpp" //----------------------------GraphKit----------------------------------------- // Main utility constructor. diff -r f79a8217d4c9 -r 5d462d4b7a8b src/hotspot/share/opto/parse1.cpp --- a/src/hotspot/share/opto/parse1.cpp Tue Oct 22 11:55:58 2019 +0200 +++ b/src/hotspot/share/opto/parse1.cpp Thu Nov 07 10:28:29 2019 +0000 @@ -41,6 +41,7 @@ #include "runtime/handles.inline.hpp" #include "runtime/safepointMechanism.hpp" #include "runtime/sharedRuntime.hpp" +#include "utilities/bitMap.inline.hpp" #include "utilities/copy.hpp" // Static array so we can figure out which bytecodes stop us from compiling diff -r f79a8217d4c9 -r 5d462d4b7a8b src/hotspot/share/utilities/bitMap.hpp --- a/src/hotspot/share/utilities/bitMap.hpp Tue Oct 22 11:55:58 2019 +0200 +++ b/src/hotspot/share/utilities/bitMap.hpp Thu Nov 07 10:28:29 2019 +0000 @@ -26,6 +26,7 @@ #define SHARE_UTILITIES_BITMAP_HPP #include "memory/allocation.hpp" +#include "runtime/atomic.hpp" #include "utilities/align.hpp" // Forward decl; @@ -105,6 +106,8 @@ void set_word (idx_t word) { set_word(word, ~(bm_word_t)0); } void clear_word(idx_t word) { _map[word] = 0; } + static inline const bm_word_t load_word_ordered(const volatile bm_word_t* const addr, atomic_memory_order memory_order); + // Utilities for ranges of bits. Ranges are half-open [beg, end). // Ranges within a single word. @@ -204,6 +207,9 @@ return (*word_addr(index) & bit_mask(index)) != 0; } + // memory_order must be memory_order_relaxed or memory_order_acquire. + bool par_at(idx_t index, atomic_memory_order memory_order = memory_order_acquire) const; + // Align bit index up or down to the next bitmap word boundary, or check // alignment. static idx_t word_align_up(idx_t bit) { @@ -220,9 +226,14 @@ inline void set_bit(idx_t bit); inline void clear_bit(idx_t bit); - // Atomically set or clear the specified bit. - inline bool par_set_bit(idx_t bit); - inline bool par_clear_bit(idx_t bit); + // Attempts to change a bit to a desired value. The operation returns true if + // this thread changed the value of the bit. It was changed with a RMW operation + // using the specified memory_order. The operation returns false if the change + // could not be set due to the bit already being observed in the desired state. + // The atomic access that observed the bit in the desired state has acquire + // semantics, unless memory_order is memory_order_relaxed or memory_order_release. + inline bool par_set_bit(idx_t bit, atomic_memory_order memory_order = memory_order_conservative); + inline bool par_clear_bit(idx_t bit, atomic_memory_order memory_order = memory_order_conservative); // Put the given value at the given offset. The parallel version // will CAS the value into the bitmap and is quite a bit slower. diff -r f79a8217d4c9 -r 5d462d4b7a8b src/hotspot/share/utilities/bitMap.inline.hpp --- a/src/hotspot/share/utilities/bitMap.inline.hpp Tue Oct 22 11:55:58 2019 +0200 +++ b/src/hotspot/share/utilities/bitMap.inline.hpp Thu Nov 07 10:28:29 2019 +0000 @@ -26,6 +26,7 @@ #define SHARE_UTILITIES_BITMAP_INLINE_HPP #include "runtime/atomic.hpp" +#include "runtime/orderAccess.hpp" #include "utilities/bitMap.hpp" #include "utilities/count_trailing_zeros.hpp" @@ -39,18 +40,39 @@ *word_addr(bit) &= ~bit_mask(bit); } -inline bool BitMap::par_set_bit(idx_t bit) { +inline const BitMap::bm_word_t BitMap::load_word_ordered(const volatile bm_word_t* const addr, atomic_memory_order memory_order) { + if (memory_order == memory_order_relaxed || memory_order == memory_order_release) { + return Atomic::load(addr); + } else { + assert(memory_order == memory_order_acq_rel || + memory_order == memory_order_acquire || + memory_order == memory_order_conservative, + "unexpected memory ordering"); + return OrderAccess::load_acquire(addr); + } +} + +inline bool BitMap::par_at(idx_t index, atomic_memory_order memory_order) const { + verify_index(index); + assert(memory_order == memory_order_acquire || + memory_order == memory_order_relaxed, + "unexpected memory ordering"); + const volatile bm_word_t* const addr = word_addr(index); + return (load_word_ordered(addr, memory_order) & bit_mask(index)) != 0; +} + +inline bool BitMap::par_set_bit(idx_t bit, atomic_memory_order memory_order) { verify_index(bit); volatile bm_word_t* const addr = word_addr(bit); const bm_word_t mask = bit_mask(bit); - bm_word_t old_val = *addr; + bm_word_t old_val = load_word_ordered(addr, memory_order); do { const bm_word_t new_val = old_val | mask; if (new_val == old_val) { return false; // Someone else beat us to it. } - const bm_word_t cur_val = Atomic::cmpxchg(new_val, addr, old_val); + const bm_word_t cur_val = Atomic::cmpxchg(new_val, addr, old_val, memory_order); if (cur_val == old_val) { return true; // Success. } @@ -58,18 +80,18 @@ } while (true); } -inline bool BitMap::par_clear_bit(idx_t bit) { +inline bool BitMap::par_clear_bit(idx_t bit, atomic_memory_order memory_order) { verify_index(bit); volatile bm_word_t* const addr = word_addr(bit); const bm_word_t mask = ~bit_mask(bit); - bm_word_t old_val = *addr; + bm_word_t old_val = load_word_ordered(addr, memory_order); do { const bm_word_t new_val = old_val & mask; if (new_val == old_val) { return false; // Someone else beat us to it. } - const bm_word_t cur_val = Atomic::cmpxchg(new_val, addr, old_val); + const bm_word_t cur_val = Atomic::cmpxchg(new_val, addr, old_val, memory_order); if (cur_val == old_val) { return true; // Success. }