RFR: 8293117: Add atomic bitset functions [v2]

Kim Barrett kbarrett at openjdk.org
Sat Apr 29 01:12:53 UTC 2023


On Fri, 28 Apr 2023 08:22:49 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   only run 64bit tests on 64bit platforms
>
> src/hotspot/share/runtime/atomic.hpp line 703:
> 
>> 701:       old_value = fetched_value;
>> 702:       new_value = operation(old_value);
>> 703:       fetched_value = Atomic::cmpxchg(dest, old_value, new_value, order);
> 
> At some point in future, it would be nice to replace the retry-loops like these with _weak_ CASes. It could be done in platform-specific code, though, but it would seem awkward to specialize for all LL/SC-retry-loop platforms just to get access to weak CAS.

Yes.  I haven't checked, but I would guess the majority of our CAS uses are loops that would benefit from
using a weak CAS on platforms where that's available.

> src/hotspot/share/runtime/atomic.hpp line 775:
> 
>> 773:     return bits ^ Atomic::fetch_then_xor(dest, bits, order);
>> 774:   }
>> 775: };
> 
> Is this used somewhere? Looks like all current helpers use CASes instead of this "prefetch" code.

Not used yet.  It's used by some of the platform-specific implementations I have queued up.

> test/hotspot/gtest/runtime/test_atomic.cpp line 287:
> 
>> 285: TEST(AtomicBitopsTest, int64) {
>> 286:   AtomicBitopsTestSupport<int64_t>()();
>> 287: }
> 
> These 64-bit tests would fail on 32-bit systems, are they not? They are outside the requirements for the contract to only support `sizeof(int)` and `sizeof(void*`). Probably just `#ifdef _LP64` these?

Well-spotted.  I forgot to conditionalize for `_LP64` as is done for other tests in this file.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13711#discussion_r1180898539
PR Review Comment: https://git.openjdk.org/jdk/pull/13711#discussion_r1180898544
PR Review Comment: https://git.openjdk.org/jdk/pull/13711#discussion_r1180898563


More information about the hotspot-runtime-dev mailing list