RFR: 8307533: Use atomic bitset functions for metadata flags
Kim Barrett
kbarrett at openjdk.org
Sat May 6 05:42:18 UTC 2023
On Fri, 5 May 2023 19:58:49 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
> Replace the bit set copies from metadata to use the Atomic functions.
> Tested with tier1-4.
Changes requested by kbarrett (Reviewer).
src/hotspot/share/oops/fieldInfo.inline.hpp line 159:
> 157:
> 158: inline void FieldStatus::atomic_clear_bits(u1& flags, u1 mask) {
> 159: u1 val = (~mask);
Why introduce a new variable?
src/hotspot/share/oops/fieldInfo.inline.hpp line 160:
> 158: inline void FieldStatus::atomic_clear_bits(u1& flags, u1 mask) {
> 159: u1 val = (~mask);
> 160: Atomic::fetch_then_and(&flags, val);
u1 is not a supported type for Atomic bitops. This only happens to work right now because all
platforms are currently using a cmpxchg-based implementation and aren't enforcing the documented
limitation of only providing support for size of an int or size of a pointer (if different).
src/hotspot/share/oops/instanceKlassFlags.hpp line 127:
> 125:
> 126: void atomic_set_bits(u1 bits) { Atomic::fetch_then_or(&_status, bits); }
> 127: void atomic_clear_bits(u1 bits) { u1 val = (~bits); Atomic::fetch_then_and(&_status, val); }
Again here, u1 is not a supported type for Atomic bitops.
src/hotspot/share/oops/instanceKlassFlags.hpp line 127:
> 125:
> 126: void atomic_set_bits(u1 bits) { Atomic::fetch_then_or(&_status, bits); }
> 127: void atomic_clear_bits(u1 bits) { u1 val = (~bits); Atomic::fetch_then_and(&_status, val); }
Why introduce a new variable?
src/hotspot/share/oops/methodFlags.hpp line 91:
> 89: int as_int() const { return _status; }
> 90: void atomic_set_bits(u4 bits) { Atomic::fetch_then_or(&_status, bits); }
> 91: void atomic_clear_bits(u4 bits) { u4 val = (~bits); Atomic::fetch_then_and(&_status, val); }
Why introduce a new variable (and why the extra parens). Just
Atomic::fetch_then_and(&_status, ~bits);
-------------
PR Review: https://git.openjdk.org/jdk/pull/13843#pullrequestreview-1415699630
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641849
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641272
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641459
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641809
PR Review Comment: https://git.openjdk.org/jdk/pull/13843#discussion_r1186641760
More information about the hotspot-dev
mailing list