From 4fd61ad1b09e2dcb25166ff906309d272f623418 Mon Sep 17 00:00:00 2001 From: Alexander Shabalin Date: Fri, 3 Dec 2021 17:08:21 +0000 Subject: [PATCH] [K/N] Change AtomicReference for the new MM ^KT-50026 Make AtomicReference behave like FreezableAtomicReference with the new MM: not frozen by default, don't require freezing value unless the ref itself is frozen. The behaviour with the old MM is unchanged. Merge-request: KT-MR-5155 --- .../kotlin/backend/konan/KonanFqNames.kt | 1 + .../konan/descriptors/DescriptorUtils.kt | 17 +++++--- .../kotlin/backend/konan/llvm/IrToBitcode.kt | 35 +++++++++-------- .../backend/konan/llvm/LlvmDeclarations.kt | 4 +- .../backend/konan/llvm/RTTIGenerator.kt | 2 +- .../konan/lower/FileInitializersLowering.kt | 2 +- .../backend/konan/optimizations/DataFlowIR.kt | 4 +- .../tests/runtime/workers/atomic0.kt | 39 +++++++++++++++++-- .../kotlin/native/concurrent/Atomics.kt | 6 ++- .../kotlin/native/internal/Annotations.kt | 7 ++++ 10 files changed, 84 insertions(+), 33 deletions(-) diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanFqNames.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanFqNames.kt index bf2e50511c0..4bcc4312d83 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanFqNames.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanFqNames.kt @@ -26,6 +26,7 @@ object KonanFqNames { val threadLocal = FqName("kotlin.native.concurrent.ThreadLocal") val sharedImmutable = FqName("kotlin.native.concurrent.SharedImmutable") val frozen = FqName("kotlin.native.internal.Frozen") + val frozenLegacyMM = FqName("kotlin.native.internal.FrozenLegacyMM") val leakDetectorCandidate = FqName("kotlin.native.internal.LeakDetectorCandidate") val canBePrecreated = FqName("kotlin.native.internal.CanBePrecreated") val typedIntrinsic = FqName("kotlin.native.internal.TypedIntrinsic") diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/descriptors/DescriptorUtils.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/descriptors/DescriptorUtils.kt index 6e5ce225749..7916b9ccae8 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/descriptors/DescriptorUtils.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/descriptors/DescriptorUtils.kt @@ -265,10 +265,17 @@ fun IrDeclaration.findTopLevelDeclaration(): IrDeclaration = when { (this.parent as IrDeclaration).findTopLevelDeclaration() } -internal val IrClass.isFrozen: Boolean - get() = annotations.hasAnnotation(KonanFqNames.frozen) || - // RTTI is used for non-reference type box: - !this.defaultType.binaryTypeIsReference() +internal fun IrClass.isFrozen(context: Context): Boolean { + val isLegacyMM = context.memoryModel != MemoryModel.EXPERIMENTAL + return when { + !context.config.freezing.freezeImplicit -> false + annotations.hasAnnotation(KonanFqNames.frozen) -> true + annotations.hasAnnotation(KonanFqNames.frozenLegacyMM) && isLegacyMM -> true + // RTTI is used for non-reference type box: + !this.defaultType.binaryTypeIsReference() -> true + else -> false + } +} fun IrConstructorCall.getAnnotationStringValue() = getValueArgument(0).safeAs>()?.value @@ -305,4 +312,4 @@ fun IrFunction.externalSymbolOrThrow(): String? { val IrFunction.isBuiltInOperator get() = origin == IrBuiltIns.BUILTIN_OPERATOR fun IrDeclaration.isFromMetadataInteropLibrary() = - descriptor.module.isFromInteropLibrary() \ No newline at end of file + descriptor.module.isFromInteropLibrary() diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt index 9905b5bdcee..6946b3b9ac0 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/IrToBitcode.kt @@ -56,16 +56,20 @@ internal enum class ObjectStorageKind { } // TODO: maybe unannotated singleton objects shall be accessed from main thread only as well? -internal val IrField.storageKind: FieldStorageKind get() { +internal fun IrField.storageKind(context: Context): FieldStorageKind { // TODO: Is this correct? val annotations = correspondingPropertySymbol?.owner?.annotations ?: annotations + val isLegacyMM = context.memoryModel != MemoryModel.EXPERIMENTAL + // TODO: simplify, once IR types are fully there. + val typeAnnotations = (type.classifierOrNull?.owner as? IrAnnotationContainer)?.annotations + val typeFrozen = typeAnnotations?.hasAnnotation(KonanFqNames.frozen) == true || + (typeAnnotations?.hasAnnotation(KonanFqNames.frozenLegacyMM) == true && isLegacyMM) return when { annotations.hasAnnotation(KonanFqNames.threadLocal) -> FieldStorageKind.THREAD_LOCAL + !isLegacyMM && !context.config.freezing.freezeImplicit -> FieldStorageKind.GLOBAL !isFinal -> FieldStorageKind.GLOBAL annotations.hasAnnotation(KonanFqNames.sharedImmutable) -> FieldStorageKind.SHARED_FROZEN - // TODO: simplify, once IR types are fully there. - (type.classifierOrNull?.owner as? IrAnnotationContainer) - ?.annotations?.hasAnnotation(KonanFqNames.frozen) == true -> FieldStorageKind.SHARED_FROZEN + typeFrozen -> FieldStorageKind.SHARED_FROZEN else -> FieldStorageKind.GLOBAL } } @@ -83,15 +87,14 @@ internal fun IrClass.storageKind(context: Context): ObjectStorageKind = when { else -> ObjectStorageKind.SHARED } -val IrField.isGlobalNonPrimitive get() = when { +internal fun IrField.isGlobalNonPrimitive(context: Context) = when { type.computePrimitiveBinaryTypeOrNull() != null -> false - else -> storageKind == FieldStorageKind.GLOBAL + else -> storageKind(context) == FieldStorageKind.GLOBAL } internal fun IrField.shouldBeFrozen(context: Context): Boolean = - this.storageKind == FieldStorageKind.SHARED_FROZEN && - (context.memoryModel != MemoryModel.EXPERIMENTAL || context.config.freezing.freezeImplicit) + this.storageKind(context) == FieldStorageKind.SHARED_FROZEN internal class RTTIGeneratorVisitor(context: Context) : IrElementVisitorVoid { val generator = RTTIGenerator(context) @@ -376,7 +379,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map - if (irField.type.binaryTypeIsReference() && irField.storageKind != FieldStorageKind.THREAD_LOCAL) { + if (irField.type.binaryTypeIsReference() && irField.storageKind(context) != FieldStorageKind.THREAD_LOCAL) { val address = context.llvmDeclarations.forStaticField(irField).storageAddressAccess.getAddress( functionGenerationContext ) @@ -1687,7 +1690,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map).llvm } else { - if (context.config.threadsAreAllowed && value.symbol.owner.isGlobalNonPrimitive) { + if (context.config.threadsAreAllowed && value.symbol.owner.isGlobalNonPrimitive(context)) { functionGenerationContext.checkGlobalsAccessible(currentCodeContext.exceptionHandler) } val ptr = context.llvmDeclarations.forStaticField(value.symbol.owner).storageAddressAccess.getAddress( @@ -1706,7 +1709,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map) { assertEquals(seen.size, workers.size) } -fun test4() { +fun test4LegacyMM() { assertFailsWith { AtomicReference(Data(1)) } @@ -83,7 +83,25 @@ fun test4() { } } -fun test5() { +fun test4() { + run { + val ref = AtomicReference(Data(1)) + assertEquals(1, ref.value.value) + } + run { + val ref = AtomicReference(null) + ref.compareAndSwap(null, Data(2)) + assertEquals(2, ref.value!!.value) + } + run { + val ref = AtomicReference(null).freeze() + assertFailsWith { + ref.compareAndSwap(null, Data(2)) + } + } +} + +fun test5LegacyMM() { assertFailsWith { AtomicReference(null).value = Data(2) } @@ -94,6 +112,14 @@ fun test5() { assertEquals(3, ref.value!!.value) } +fun test5() { + val ref = AtomicReference(null) + ref.value = Data(2) + assertEquals(2, ref.value!!.value) + ref.value = Data(3).freeze() + assertEquals(3, ref.value!!.value) +} + fun test6() { val int = AtomicInt(0) int.value = 239 @@ -128,8 +154,13 @@ fun test7() { test1(workers) test2(workers) test3(workers) - test4() - test5() + if (Platform.memoryModel == MemoryModel.EXPERIMENTAL) { + test4() + test5() + } else { + test4LegacyMM() + test5LegacyMM() + } test6() test7() diff --git a/kotlin-native/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt b/kotlin-native/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt index 9ae8b309d6f..12e36f5b113 100644 --- a/kotlin-native/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt +++ b/kotlin-native/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt @@ -215,7 +215,7 @@ private fun debugString(value: Any?): String { * Otherwise memory leak could happen. To detect such leaks [kotlin.native.internal.GC.detectCycles] * in debug mode could be helpful. */ -@Frozen +@FrozenLegacyMM @LeakDetectorCandidate @NoReorderFields public class AtomicReference { @@ -232,7 +232,9 @@ public class AtomicReference { * @throws InvalidMutabilityException if reference is not frozen. */ constructor(value: T) { - checkIfFrozen(value) + if (this.isFrozen) { + checkIfFrozen(value) + } value_ = value } diff --git a/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/Annotations.kt b/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/Annotations.kt index c887af6fd2d..c55c718bd0f 100644 --- a/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/Annotations.kt +++ b/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/Annotations.kt @@ -48,6 +48,13 @@ public annotation class ExportForCompiler @Retention(AnnotationRetention.BINARY) internal annotation class Frozen +/** + * Similar to `@Frozen`, but works only for legacy MM. On the new MM this has no effect. + */ +@Target(AnnotationTarget.CLASS) +@Retention(AnnotationRetention.BINARY) +internal annotation class FrozenLegacyMM + /** * Fields of annotated class won't be sorted. */