Skip to content

Commit

Permalink
8326936: RISC-V: Shenandoah GC crashes due to incorrect atomic memory…
Browse files Browse the repository at this point in the history
… operations

8316186: RISC-V: Remove PlatformCmpxchg<4>
8330242: RISC-V: Simplify and remove CORRECT_COMPILER_ATOMIC_SUPPORT in atomic_linux_riscv.hpp

Reviewed-by: fyang
Backport-of: a089ed2b9289eeda73bba47ac87e5bc81a4af9dc
  • Loading branch information
zifeihan authored and RealFYang committed Apr 19, 2024
1 parent 7743b6c commit f0f2e70
Showing 1 changed file with 42 additions and 37 deletions.
79 changes: 42 additions & 37 deletions src/hotspot/os_cpu/linux_riscv/atomic_linux_riscv.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

#if defined(__clang_major__)
#define FULL_COMPILER_ATOMIC_SUPPORT
#elif (__GNUC__ > 13) || ((__GNUC__ == 13) && (__GNUC_MINOR__ >= 2))
#elif (__GNUC__ > 13) || ((__GNUC__ == 13) && (__GNUC_MINOR__ > 2))
#define FULL_COMPILER_ATOMIC_SUPPORT
#endif

Expand Down Expand Up @@ -114,6 +114,44 @@ inline T Atomic::PlatformCmpxchg<1>::operator()(T volatile* dest __attribute__((
}
#endif

#ifndef FULL_COMPILER_ATOMIC_SUPPORT
// The implementation of `__atomic_compare_exchange` lacks sign extensions
// in GCC 13.2 and lower when using with 32-bit unsigned integers on RV64,
// so we should implement it manually.
// GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114130.
// See also JDK-8326936.
template<>
template<typename T>
inline T Atomic::PlatformCmpxchg<4>::operator()(T volatile* dest __attribute__((unused)),
T compare_value,
T exchange_value,
atomic_memory_order order) const {
STATIC_ASSERT(4 == sizeof(T));

int32_t old_value;
uint64_t rc_temp;

if (order != memory_order_relaxed) {
FULL_MEM_BARRIER;
}

__asm__ __volatile__ (
"1: lr.w %0, %2 \n\t"
" bne %0, %3, 2f \n\t"
" sc.w %1, %4, %2 \n\t"
" bnez %1, 1b \n\t"
"2: \n\t"
: /*%0*/"=&r" (old_value), /*%1*/"=&r" (rc_temp), /*%2*/"+A" (*dest)
: /*%3*/"r" ((int64_t)(int32_t)compare_value), /*%4*/"r" (exchange_value)
: "memory" );

if (order != memory_order_relaxed) {
FULL_MEM_BARRIER;
}
return (T)old_value;
}
#endif

template<size_t byte_size>
template<typename T>
inline T Atomic::PlatformXchg<byte_size>::operator()(T volatile* dest,
Expand Down Expand Up @@ -148,54 +186,21 @@ inline T Atomic::PlatformCmpxchg<byte_size>::operator()(T volatile* dest __attri
atomic_memory_order order) const {

#ifndef FULL_COMPILER_ATOMIC_SUPPORT
STATIC_ASSERT(byte_size >= 4);
STATIC_ASSERT(byte_size > 4);
#endif

STATIC_ASSERT(byte_size == sizeof(T));
T value = compare_value;
if (order != memory_order_relaxed) {
FULL_MEM_BARRIER;
}

__atomic_compare_exchange(dest, &value, &exchange_value, /* weak */ false,
__atomic_compare_exchange(dest, &compare_value, &exchange_value, /* weak */ false,
__ATOMIC_RELAXED, __ATOMIC_RELAXED);

if (order != memory_order_relaxed) {
FULL_MEM_BARRIER;
}
return value;
}

template<>
template<typename T>
inline T Atomic::PlatformCmpxchg<4>::operator()(T volatile* dest __attribute__((unused)),
T compare_value,
T exchange_value,
atomic_memory_order order) const {
STATIC_ASSERT(4 == sizeof(T));

T old_value;
long rc;

if (order != memory_order_relaxed) {
FULL_MEM_BARRIER;
}

__asm__ __volatile__ (
"1: sext.w %1, %3 \n\t" // sign-extend compare_value
" lr.w %0, %2 \n\t"
" bne %0, %1, 2f \n\t"
" sc.w %1, %4, %2 \n\t"
" bnez %1, 1b \n\t"
"2: \n\t"
: /*%0*/"=&r" (old_value), /*%1*/"=&r" (rc), /*%2*/"+A" (*dest)
: /*%3*/"r" (compare_value), /*%4*/"r" (exchange_value)
: "memory" );

if (order != memory_order_relaxed) {
FULL_MEM_BARRIER;
}
return old_value;
return compare_value;
}

template<size_t byte_size>
Expand Down

1 comment on commit f0f2e70

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.