From a082aeb29ec5261184b7a941e2858eef52381d21 Mon Sep 17 00:00:00 2001 From: Pavel Kunyavskiy Date: Mon, 1 Aug 2022 11:17:30 +0200 Subject: [PATCH] [K/N] Make atomic ordering weaker for less performance harm --- .../kotlin/backend/konan/KonanConfig.kt | 5 +++-- .../backend/konan/llvm/CodeGenerator.kt | 19 +++++++++++++++---- kotlin-native/runtime/src/main/cpp/Common.h | 5 +++++ kotlin-native/runtime/src/main/cpp/Memory.h | 18 +++++++++++++++++- .../runtime/src/mm/cpp/ExtraObjectData.hpp | 3 ++- .../konan/target/KonanTargetExtenstions.kt | 14 +++++++++++++- 6 files changed, 55 insertions(+), 9 deletions(-) diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfig.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfig.kt index c5cd930dbe7..25d86ba8ab3 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfig.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/KonanConfig.kt @@ -1,6 +1,6 @@ /* - * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license - * that can be found in the LICENSE file. + * Copyright 2010-2022 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. */ package org.jetbrains.kotlin.backend.konan @@ -52,6 +52,7 @@ class KonanConfig(val project: Project, val configuration: CompilerConfiguration private val platformManager = PlatformManager(distribution) internal val targetManager = platformManager.targetManager(configuration.get(KonanConfigKeys.TARGET)) internal val target = targetManager.target + val targetHasAddressDependency get() = target.hasAddressDependencyInMemoryModel() internal val phaseConfig = configuration.get(CLIConfigurationKeys.PHASE_CONFIG)!! // TODO: debug info generation mode and debug/release variant selection probably requires some refactoring. diff --git a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt index 581ae745089..50518c23e62 100644 --- a/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt +++ b/kotlin-native/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt @@ -1,6 +1,6 @@ /* - * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license - * that can be found in the LICENSE file. + * Copyright 2010-2022 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. */ package org.jetbrains.kotlin.backend.konan.llvm @@ -1144,13 +1144,24 @@ internal abstract class FunctionGenerationContext( fun loadTypeInfo(objPtr: LLVMValueRef): LLVMValueRef { val typeInfoOrMetaPtr = structGep(objPtr, 0 /* typeInfoOrMeta_ */) - val typeInfoOrMetaWithFlags = load(typeInfoOrMetaPtr, memoryOrder = LLVMAtomicOrdering.LLVMAtomicOrderingAcquire) + + val memoryOrder = if (context.config.targetHasAddressDependency) { + /** + * Formally, this ordering is too weak, and doesn't prevent data race with installing extra object. + * Check comment in ObjHeader::type_info for details. + */ + LLVMAtomicOrdering.LLVMAtomicOrderingMonotonic + } else { + LLVMAtomicOrdering.LLVMAtomicOrderingAcquire + } + + val typeInfoOrMetaWithFlags = load(typeInfoOrMetaPtr, memoryOrder = memoryOrder) // Clear two lower bits. val typeInfoOrMetaWithFlagsRaw = ptrToInt(typeInfoOrMetaWithFlags, codegen.intPtrType) val typeInfoOrMetaRaw = and(typeInfoOrMetaWithFlagsRaw, codegen.immTypeInfoMask) val typeInfoOrMeta = intToPtr(typeInfoOrMetaRaw, kTypeInfoPtr) val typeInfoPtrPtr = structGep(typeInfoOrMeta, 0 /* typeInfo */) - return load(typeInfoPtrPtr) + return load(typeInfoPtrPtr, memoryOrder = LLVMAtomicOrdering.LLVMAtomicOrderingMonotonic) } fun lookupInterfaceTableRecord(typeInfo: LLVMValueRef, interfaceId: Int): LLVMValueRef { diff --git a/kotlin-native/runtime/src/main/cpp/Common.h b/kotlin-native/runtime/src/main/cpp/Common.h index 4f9e315315b..ab9a544a722 100644 --- a/kotlin-native/runtime/src/main/cpp/Common.h +++ b/kotlin-native/runtime/src/main/cpp/Common.h @@ -52,4 +52,9 @@ #define KONAN_TYPE_INFO_HAS_WRITABLE_PART 1 #endif +// should be consistent with hasAddressDependencyInMemoryModel function +#if defined(KONAN_X64) || defined(KONAN_X86) || defined(KONAN_ARM32) || defined(KONAN_ARM64) +#define KONAN_TARGET_HAS_ADDRESS_DEPENDENCY 1 +#endif + #endif // RUNTIME_COMMON_H diff --git a/kotlin-native/runtime/src/main/cpp/Memory.h b/kotlin-native/runtime/src/main/cpp/Memory.h index 84f4ab9500d..38404031a6d 100644 --- a/kotlin-native/runtime/src/main/cpp/Memory.h +++ b/kotlin-native/runtime/src/main/cpp/Memory.h @@ -62,8 +62,24 @@ struct ObjHeader { TypeInfo* typeInfoOrMetaRelaxed() const { return atomicGetRelaxed(&typeInfoOrMeta_);} TypeInfo* typeInfoOrMetaAcquire() const { return atomicGetAcquire(&typeInfoOrMeta_);} + /** + * Formally, this code data races with installing ExtraObject. Even though, we are okey, with reading + * both typeInfo and meta-object pointer, llvm memory model doesn't guarantee, that if we are able to + * see metaObject, written by other thread, we would be able to see metaObject->typeInfo. + * + * To make this correct with llvm memory model we need to use [LLVMAtomicOrdering.LLVMAtomicOrderingAcquire] here. + * Unfortunately, this is dramatically harmful for performance on arm architecture. So, we are using + * [LLVMAtomicOrdering.LLVMAtomicOrderingMonotonic] for both this read and following load of metaObject->typeInfo. + * At this point, we have no data race, but llvm memory model allows uninitialized value to be read from metaObject->typeInfo. + * + * Hardware guaranties on many supported platforms doesn't allow this to happen. + */ const TypeInfo* type_info() const { - return clearPointerBits(typeInfoOrMetaAcquire(), OBJECT_TAG_MASK)->typeInfo_; +#ifdef KONAN_TARGET_HAS_ADDRESS_DEPENDENCY + return atomicGetRelaxed(&clearPointerBits(typeInfoOrMetaRelaxed(), OBJECT_TAG_MASK)->typeInfo_); +#else + return atomicGetRelaxed(&clearPointerBits(typeInfoOrMetaAcquire(), OBJECT_TAG_MASK)->typeInfo_); +#endif } bool has_meta_object() const { diff --git a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp index 9bb3918fba2..847beaf6d8b 100644 --- a/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ExtraObjectData.hpp @@ -78,7 +78,8 @@ public: // info must be equal to objHeader->type_info(), but it needs to be loaded in advance to avoid data races explicit ExtraObjectData(ObjHeader* objHeader, const TypeInfo *info) noexcept : - typeInfo_(info), weakReferenceCounterOrBaseObject_(objHeader) { + typeInfo_(nullptr), weakReferenceCounterOrBaseObject_(objHeader) { + atomicSetRelease(&typeInfo_, info); } ~ExtraObjectData(); private: diff --git a/kotlin-native/shared/src/main/kotlin/org/jetbrains/kotlin/konan/target/KonanTargetExtenstions.kt b/kotlin-native/shared/src/main/kotlin/org/jetbrains/kotlin/konan/target/KonanTargetExtenstions.kt index baceec23fe6..eb009688e49 100644 --- a/kotlin-native/shared/src/main/kotlin/org/jetbrains/kotlin/konan/target/KonanTargetExtenstions.kt +++ b/kotlin-native/shared/src/main/kotlin/org/jetbrains/kotlin/konan/target/KonanTargetExtenstions.kt @@ -1,3 +1,8 @@ +/* + * Copyright 2010-2022 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + package org.jetbrains.kotlin.konan.target // TODO: This all needs to go to konan.properties @@ -17,7 +22,7 @@ fun KonanTarget.supportsMimallocAllocator(): Boolean = is KonanTarget.MINGW_X86 -> true is KonanTarget.MINGW_X64 -> true is KonanTarget.MACOS_X64 -> true - is KonanTarget.MACOS_ARM64 -> true + is KonanTarget.MACOS_ARM64 -> true is KonanTarget.LINUX_ARM64 -> true is KonanTarget.LINUX_ARM32_HFP -> true is KonanTarget.ANDROID_X64 -> true @@ -59,3 +64,10 @@ fun KonanTarget.supportedSanitizers(): List = // TODO: Support macOS arm64 else -> listOf() } + +// should be consistent with KONAN_TARGET_HAS_ADDRESS_DEPENDENCY macro +fun KonanTarget.hasAddressDependencyInMemoryModel(): Boolean = + when (this.architecture) { + Architecture.X86, Architecture.X64, Architecture.ARM32, Architecture.ARM64 -> true + Architecture.MIPS32, Architecture.MIPSEL32, Architecture.WASM32 -> false + }