[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-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")

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
+internal fun IrClass.isFrozen(context: Context): Boolean {
-    get() = annotations.hasAnnotation(KonanFqNames.frozen) ||
+    val isLegacyMM = context.memoryModel != MemoryModel.EXPERIMENTAL
-            // RTTI is used for non-reference type box:
+    return when {
-            !this.defaultType.binaryTypeIsReference()
+        !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<IrConst<String>>()?.value
 
@@ -305,4 +312,4 @@ fun IrFunction.externalSymbolOrThrow(): String? {
 val IrFunction.isBuiltInOperator get() = origin == IrBuiltIns.BUILTIN_OPERATOR
 
 fun IrDeclaration.isFromMetadataInteropLibrary() =
-        descriptor.module.isFromInteropLibrary()
+        descriptor.module.isFromInteropLibrary()

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.
+        typeFrozen -> FieldStorageKind.SHARED_FROZEN
-        (type.classifierOrNull?.owner as? IrAnnotationContainer)
-                ?.annotations?.hasAnnotation(KonanFqNames.frozen) == true -> 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 &&
+        this.storageKind(context) == FieldStorageKind.SHARED_FROZEN
-                (context.memoryModel != MemoryModel.EXPERIMENTAL || context.config.freezing.freezeImplicit)
 
 internal class RTTIGeneratorVisitor(context: Context) : IrElementVisitorVoid {
     val generator = RTTIGenerator(context)
@@ -376,7 +379,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
                     using(parameterScope) usingParameterScope@{
                         using(VariableScope()) usingVariableScope@{
                             context.llvm.initializersGenerationState.topLevelFields
-                                    .filter { it.storageKind != FieldStorageKind.THREAD_LOCAL }
+                                    .filter { it.storageKind(context) != FieldStorageKind.THREAD_LOCAL }
                                     .filterNot { it.shouldBeInitializedEagerly }
                                     .forEach { initGlobalField(it) }
                             ret(null)
@@ -393,7 +396,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
                     using(parameterScope) usingParameterScope@{
                         using(VariableScope()) usingVariableScope@{
                             context.llvm.initializersGenerationState.topLevelFields
-                                    .filter { it.storageKind == FieldStorageKind.THREAD_LOCAL }
+                                    .filter { it.storageKind(context) == FieldStorageKind.THREAD_LOCAL }
                                     .filterNot { it.shouldBeInitializedEagerly }
                                     .forEach { initThreadLocalField(it) }
                             ret(null)
@@ -494,7 +497,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
                 appendingTo(bbInit) {
                     context.llvm.initializersGenerationState.topLevelFields
                             .filter { !context.useLazyFileInitializers() || it.shouldBeInitializedEagerly }
-                            .filterNot { it.storageKind == FieldStorageKind.THREAD_LOCAL }
+                            .filterNot { it.storageKind(context) == FieldStorageKind.THREAD_LOCAL }
                             .forEach { initGlobalField(it) }
                     context.llvm.initializersGenerationState.moduleGlobalInitializers.forEach {
                         evaluateSimpleFunctionCall(it, emptyList(), Lifetime.IRRELEVANT)
@@ -510,7 +513,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
                     }
                     context.llvm.initializersGenerationState.topLevelFields
                             .filter { !context.useLazyFileInitializers() || it.shouldBeInitializedEagerly }
-                            .filter { it.storageKind == FieldStorageKind.THREAD_LOCAL }
+                            .filter { it.storageKind(context) == FieldStorageKind.THREAD_LOCAL }
                             .forEach { initThreadLocalField(it) }
                     context.llvm.initializersGenerationState.moduleThreadLocalInitializers.forEach {
                         evaluateSimpleFunctionCall(it, emptyList(), Lifetime.IRRELEVANT)
@@ -531,7 +534,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
                     context.llvm.initializersGenerationState.topLevelFields
                             // Only if a subject for memory management.
                             .forEach { irField ->
-                                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<IrE
             if (value.symbol.owner.correspondingPropertySymbol?.owner?.isConst == true) {
                 evaluateConst(value.symbol.owner.initializer?.expression as IrConst<*>).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<IrE
     private fun needMutationCheck(irClass: IrClass): Boolean {
         // For now we omit mutation checks on immutable types, as this allows initialization in constructor
         // and it is assumed that API doesn't allow to change them.
-        return !irClass.isFrozen
+        return !irClass.isFrozen(context)
     }
 
     private fun isZeroConstValue(value: IrExpression): Boolean {
@@ -1756,7 +1759,7 @@ internal class CodeGeneratorVisitor(val context: Context, val lifetimes: Map<IrE
             val globalAddress = context.llvmDeclarations.forStaticField(value.symbol.owner).storageAddressAccess.getAddress(
                     functionGenerationContext
             )
-            if (context.config.threadsAreAllowed && value.symbol.owner.storageKind == FieldStorageKind.GLOBAL)
+            if (context.config.threadsAreAllowed && value.symbol.owner.storageKind(context) == FieldStorageKind.GLOBAL)
                 functionGenerationContext.checkGlobalsAccessible(currentCodeContext.exceptionHandler)
             if (value.symbol.owner.shouldBeFrozen(context))
                 functionGenerationContext.freeze(valueToAssign, currentCodeContext.exceptionHandler)

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/LlvmDeclarations.kt

@@ -322,7 +322,7 @@ private class DeclarationsGeneratorVisitor(override val context: Context) :
         } else {
             // Fields are module-private, so we use internal name:
             val name = "kvar:" + qualifyInternalName(declaration)
-            val storage = if (declaration.storageKind == FieldStorageKind.THREAD_LOCAL) {
+            val storage = if (declaration.storageKind(context) == FieldStorageKind.THREAD_LOCAL) {
                 addKotlinThreadLocal(name, getLLVMType(declaration.type))
             } else {
                 addKotlinGlobal(name, getLLVMType(declaration.type), isExported = false)
@@ -416,4 +416,4 @@ private class CodegenStaticFieldMetadata(
         val llvm: StaticFieldLlvmDeclarations
 ) : KonanMetadata(name, konanLibrary), MetadataSource.Property {
     override val isConst = false
-}
+}

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/RTTIGenerator.kt

@@ -53,7 +53,7 @@ internal class RTTIGenerator(override val context: Context) : ContextUtils {
 
     private fun flagsFromClass(irClass: IrClass): Int {
         var result = 0
-        if (irClass.isFrozen && context.config.freezing.freezeImplicit)
+        if (irClass.isFrozen(context))
            result = result or TF_IMMUTABLE
         // TODO: maybe perform deeper analysis to find surely acyclic types.
         if (!irClass.isInterface && !irClass.isAbstract() && !irClass.isAnnotationClass) {

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/lower/FileInitializersLowering.kt

@@ -56,7 +56,7 @@ internal class FileInitializersLowering(val context: Context) : FileLoweringPass
         for (declaration in irFile.declarations) {
             val irField = (declaration as? IrField) ?: (declaration as? IrProperty)?.backingField
             if (irField == null || !irField.needsInitializationAtRuntime || irField.shouldBeInitializedEagerly) continue
-            if (irField.storageKind != FieldStorageKind.THREAD_LOCAL) {
+            if (irField.storageKind(context) != FieldStorageKind.THREAD_LOCAL) {
                 requireGlobalInitializer = true
             } else {
                 requireThreadLocalInitializer = true // Either marked with thread local or only main thread visible.

kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/optimizations/DataFlowIR.kt

@@ -640,7 +640,7 @@ internal object DataFlowIR {
                     val placeToFunctionsTable = !isAbstract && it !is IrConstructor && irClass != null
                             && (it.isOverridableOrOverrides || bridgeTarget != null || function.isSpecial || !irClass.isFinal())
                     val symbolTableIndex = if (placeToFunctionsTable) module.numberOfFunctions++ else -1
-                    val frozen = it is IrConstructor && irClass!!.annotations.findAnnotation(KonanFqNames.frozen) != null
+                    val frozen = it is IrConstructor && irClass!!.isFrozen(context)
                     val functionSymbol = if (it.isExported())
                         FunctionSymbol.Public(name.localHash.value, module, symbolTableIndex, attributes, it, bridgeTargetSymbol, takeName { name })
                     else
@@ -683,4 +683,4 @@ internal object DataFlowIR {
             return symbol
         }
     }
-}
+}

kotlin-native/backend.native/tests/runtime/workers/atomic0.kt

@@ -74,7 +74,7 @@ fun test3(workers: Array<Worker>) {
     assertEquals(seen.size, workers.size)
 }
 
-fun test4() {
+fun test4LegacyMM() {
     assertFailsWith<InvalidMutabilityException> {
         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<Data?>(null)
+        ref.compareAndSwap(null, Data(2))
+        assertEquals(2, ref.value!!.value)
+    }
+    run {
+        val ref = AtomicReference<Data?>(null).freeze()
+        assertFailsWith<InvalidMutabilityException> {
+            ref.compareAndSwap(null, Data(2))
+        }
+    }
+}
+
+fun test5LegacyMM() {
     assertFailsWith<InvalidMutabilityException> {
         AtomicReference<Data?>(null).value = Data(2)
     }
@@ -94,6 +112,14 @@ fun test5() {
     assertEquals(3, ref.value!!.value)
 }
 
+fun test5() {
+    val ref = AtomicReference<Data?>(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()
+    if (Platform.memoryModel == MemoryModel.EXPERIMENTAL) {
-    test5()
+        test4()
+        test5()
+    } else {
+        test4LegacyMM()
+        test5LegacyMM()
+    }
     test6()
     test7()
 

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<T> {
@@ -232,7 +232,9 @@ public class AtomicReference<T> {
      * @throws InvalidMutabilityException if reference is not frozen.
      */
     constructor(value: T) {
-        checkIfFrozen(value)
+        if (this.isFrozen) {
+            checkIfFrozen(value)
+        }
         value_ = value
     }
 

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.
  */