Preliminary tvOS support #3303

sbogolepov · 2019-09-02T11:53:35Z

No description provided.

SvyatoslavScherbina · 2019-09-02T14:07:59Z

...e/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExport.kt

        contents.append(when (target.family) {
            Family.IOS -> """
                |    <key>MinimumOSVersion</key>
                |    <string>$minimumOsVersion</string>
                |    <key>UIDeviceFamily</key>
                |    <array>
-                |        <integer>1</integer>
-                |        <integer>2</integer>
+                |       $uiDeviceFamilyValues


Will this emit proper indent for second uiDeviceFamilyValues line?

SvyatoslavScherbina · 2019-09-02T14:10:56Z

...e/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExport.kt

                |    </array>

                """.trimMargin()
            Family.OSX -> ""
            else -> error(target)
        })

-        if (target == KonanTarget.IOS_ARM64) {
+        if (target.architecture == Architecture.ARM64) {


Does Xcode emit UIRequiredDeviceCapabilities with arm64 for tvOS arm64 device framework?

Nope, it's incorrect. 👍

SvyatoslavScherbina · 2019-09-02T14:13:46Z

...native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/BitcodeEmbedding.kt

+
+    private val KonanConfig.bitcodeEmbeddingMode: Mode
+        get() = when {
+            shouldForceBitcodeEmbedding() -> if (debug) Mode.MARKER else Mode.FULL


Not sure if CLI compiler driver should do this.
Does clang enforce bitcode for tvOS?

Agree, it's a responsibility of a build system.

Does clang enforce bitcode for tvOS?

No.

SvyatoslavScherbina · 2019-09-02T14:18:33Z

backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/Linker.kt

@@ -18,6 +19,13 @@ internal fun determineLinkerOutput(context: Context): LinkerOutputKind =
            else -> TODO("${context.config.produce} should not reach native linker stage")
        }

+// TODO: Move to a more appropriate place.
+internal val KonanTarget.isAppleTarget: Boolean


This name doesn't reflect the actual conditions that need to be checked on its use sites.
This is generally more like IsThisParticularKindOfObjCInteropEnabled.
I would prefer using old target-family-based approach for now.

Unless you have short-term plans to add more relevant target families of course.

Yes, I think we should introduce Family.TVOS, Family.WATCHOS because different flavours of iOS have different platform libs and runtime libraries (compiler_rt.*). And when over all Apple targets is getting kinda cumbersome.

You mean platform libraries are deduced from the family in

kotlin-native/platformLibs/build.gradle

Line 39 in bcfd196

file("src/platform/${getVisibleName(target.family)}")

? This could be worked around (like hack for MIPS zlib), but indeed may be a good idea to use OS family as identity for the set of platform libs.

Yes, that's one of the reasons.

SvyatoslavScherbina · 2019-09-02T14:19:20Z

backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/Linker.kt

+internal val KonanTarget.isAppleTarget: Boolean
+        get() = family == Family.IOS || family == Family.OSX
+
+internal val KonanTarget.isTvOs: Boolean


Renamed to isTvOsBased. Will be removed with introduction of Family.TVOS.

No, isTvOS is good. I mean that s should probably be capital.

Ok. Anyway, isTvOsBased feels a bit better since target is not an OS :)

SvyatoslavScherbina · 2019-09-02T14:23:28Z

build-tools/src/main/kotlin/org/jetbrains/kotlin/FrameworkTest.kt

@@ -131,9 +150,10 @@ open class FrameworkTest : DefaultTask() {
        val bitcodeBuildTool = "${configurables.absoluteAdditionalToolsDir}/bin/bitcode-build-tool"
        val ldPath = "${configurables.absoluteTargetToolchain}/usr/bin/ld"
        val sdk = when (testTarget) {
-            KonanTarget.IOS_X64 -> return // bitcode-build-tool doesn't support iPhone Simulator.
+            KonanTarget.IOS_X64, KonanTarget.TVOS_X64 -> return // bitcode-build-tool doesn't support iPhone Simulator.


Comment should be updated too.

SvyatoslavScherbina · 2019-09-02T14:36:36Z

shared/src/main/kotlin/org/jetbrains/kotlin/konan/target/ClangArgs.kt

@@ -69,6 +69,12 @@ class ClangArgs(private val configurables: Configurables) : Configurables by con
                KonanTarget.IOS_ARM64 ->
                    listOf("-stdlib=libc++", "-arch", "arm64", "-isysroot", absoluteTargetSysRoot, "-miphoneos-version-min=9.0.0")

+                KonanTarget.TVOS_ARM64 ->
+                    listOf("-stdlib=libc++", "-arch", "arm64", "-isysroot", absoluteTargetSysRoot, "-mtvos-version-min=12.2")


Better use older version.

Don't we have osVersionMin.tvos_x64 and such?

SvyatoslavScherbina · 2019-09-02T15:24:52Z

build-tools/src/main/kotlin/org/jetbrains/kotlin/XcRunRuntimeUtils.kt

+        if (version1[index] < version2[index]) return -1;
+        if (version1[index] > version2[index]) return 1;
+    }
+    return when {


Is it different from

return version1.size.compareTo(version2.size)

?

SvyatoslavScherbina · 2019-09-02T15:43:57Z

shared/src/main/kotlin/org/jetbrains/kotlin/konan/target/Linker.kt

@@ -136,19 +136,22 @@ open class MacOSBasedLinker(targetProperties: AppleConfigurables)
    private val dsymutil = "$absoluteLlvmHome/bin/llvm-dsymutil"

    private fun provideCompilerRtLibrary(libraryName: String): String? {
-        val suffix = if (libraryName.isNotEmpty() && target == KonanTarget.IOS_X64) {
-            "iossim"
+        val prefix = when (val family = target.family) {


Why not just have when over targets?
This would be simpler and more discoverable.

SvyatoslavScherbina · 2019-09-02T15:49:28Z

konan/konan.properties

+
+linkerNoDebugFlags.tvos_arm64 = -S
+linkerDynamicFlags.tvos_arm64 = -dylib
+linkerKonanFlags.tvos_arm64 = -lSystem -lc++ -lobjc -framework Foundation -sdk_version 11.2


Different SDK versions for tvos_arm64 and tvos_x64.

SvyatoslavScherbina · 2019-09-02T15:51:49Z

Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/ObjCStubs.kt

@@ -247,15 +248,15 @@ internal val ObjCContainer.classOrProtocol: ObjCClassOrProtocol
 */
 internal fun Type.isStret(target: KonanTarget): Boolean {
    val unwrappedType = this.unwrapTypedefs()
-    return when (target) {
-        KonanTarget.IOS_ARM64 ->
+    return when (target.architecture) {


Checking only architecture here is wrong, as targets with same architecture can have different ABIs.

+1, guess current approach with explicit selection seems better fitting.

olonho · 2019-09-02T17:17:01Z

...nd.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/CStubsManager.kt

-        val sourceFileExtension = when (target.family) {
-            Family.OSX, Family.IOS -> {
+        val sourceFileExtension = when {
+            target.isAppleTarget -> {


Guess previous version was readable enough.

If we introduce Family.TVOS, Family.WATCHOS then it will be replaced with something like target.family.isAppleFamily/ isDarwinBased.

TVOS and WATCHOS are pretty much the same as IOS, so why not just stick this way.

olonho · 2019-09-02T17:21:52Z

build-tools/src/main/kotlin/org/jetbrains/kotlin/XcRunRuntimeUtils.kt

@@ -0,0 +1,63 @@
+package org.jetbrains.kotlin
+
+import kotlinx.serialization.Serializable


Hmm, is it indeed a good idea to depend on kotlinx.serialization. here?

We already have dependencies on it in build-utils (see ExternalReportUtils.kt) and I don't think it can cause any trouble.

olonho · 2019-09-03T06:08:53Z

What about platform libs?

sbogolepov · 2019-09-03T07:51:42Z

I suggest to add them separately to make review easier.

olonho · 2019-09-03T11:52:51Z

build-tools/src/main/kotlin/org/jetbrains/kotlin/XcRunRuntimeUtils.kt

-        version1.size == version2.size -> 0
-        version1.size < version2.size -> -1
-        else -> 1
+        if (version1[index] < version2[index]) return -1


Why not use compareTo here as well?

Because we want to continue iteration in case of equality.

SvyatoslavScherbina · 2019-09-04T07:26:22Z

shared/src/main/kotlin/org/jetbrains/kotlin/konan/target/ClangArgs.kt


                KonanTarget.IOS_ARM32 ->
-                    listOf("-stdlib=libc++", "-arch", "armv7", "-isysroot", absoluteTargetSysRoot, "-miphoneos-version-min=9.0.0")
+                    listOf("-stdlib=libc++", "-arch", "armv7", "-isysroot", absoluteTargetSysRoot, "-miphoneos-version-min=$osVersionMin")


Isn't osVersionMin actually differ from "9.0.0"?

Well, it's 9.0. What kind of problem may it cause?

SvyatoslavScherbina · 2019-09-04T07:32:40Z

...e/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExport.kt

+        // 1 - iPhone
+        // 2 - iPad
+        // 3 - AppleTV
+        val uiDeviceFamilyValues = if (target.isTvOsBased) {


This will anyway turn into when over target (or target family) after adding watchOS, making isTvOsBased unnecessary. Then why not just use this fact?

SvyatoslavScherbina · 2019-09-04T07:33:03Z

...e/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExport.kt

-                |       $uiDeviceFamilyValues
+                ${uiDeviceFamilyValues.joinToString(separator = "\n") { 
+                "|       <integer>$it</integer>" 
+            }}


Weird indent?

SvyatoslavScherbina · 2019-09-04T07:37:38Z

Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/ObjCStubs.kt

@@ -16,6 +16,7 @@

 package org.jetbrains.kotlin.native.interop.gen

+import org.jetbrains.kotlin.konan.target.Architecture


Unused import.

SvyatoslavScherbina · 2019-09-04T07:38:42Z

Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/ObjCStubs.kt

@@ -248,19 +249,24 @@ internal val ObjCContainer.classOrProtocol: ObjCClassOrProtocol
 internal fun Type.isStret(target: KonanTarget): Boolean {
    val unwrappedType = this.unwrapTypedefs()
    return when (target) {
-        KonanTarget.IOS_ARM64 ->
+        KonanTarget.IOS_ARM64,
+        KonanTarget.TVOS_ARM64 ->


Btw, have you verified that tvOS arm64 ABI doesn't use stret too?

SvyatoslavScherbina · 2019-09-04T07:39:31Z

Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/ObjCStubs.kt

+        KonanTarget.IOS_X64,
+        KonanTarget.MACOS_X64,
+        KonanTarget.WATCHOS_X64,
+        KonanTarget.TVOS_X64 -> when (unwrappedType) {


Please ensure that watchOS and tvOS simulators use regular macOS x86_64 ABI.

SvyatoslavScherbina · 2019-09-05T13:34:13Z

...e/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExport.kt

                |    <key>MinimumOSVersion</key>
                |    <string>$minimumOsVersion</string>
                |    <key>UIDeviceFamily</key>
                |    <array>
-                |        <integer>1</integer>
-                |        <integer>2</integer>
+                |$xmlValues       


The second line doesn't have | prefix. Does this work properly?

olonho · 2019-09-05T13:58:26Z

...native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/BitcodeEmbedding.kt

-            addEmptyMarker(llvm, "konan_llvm_cmdline", "__LLVM,__cmdline")
+    private val KonanConfig.bitcodeEmbeddingMode: Mode
+        get() = configuration.get(KonanConfigKeys.BITCODE_EMBEDDING_MODE)!!.also {
+            // TODO: We cannot produce programs due to `-alias` linked flag.


Hmm, so why not fix alias with LLVM API call as suggested in #3288?

SvyatoslavScherbina reviewed Sep 2, 2019

View reviewed changes

olonho reviewed Sep 2, 2019

View reviewed changes

sbogolepov force-pushed the tvos_support branch from ee535d2 to 34742a1 Compare September 3, 2019 08:51

olonho reviewed Sep 3, 2019

View reviewed changes

SvyatoslavScherbina reviewed Sep 4, 2019

View reviewed changes

sbogolepov force-pushed the tvos_support branch 3 times, most recently from 9245a93 to a47d3c6 Compare September 5, 2019 12:23

SvyatoslavScherbina reviewed Sep 5, 2019

View reviewed changes

olonho reviewed Sep 5, 2019

View reviewed changes

SvyatoslavScherbina approved these changes Sep 6, 2019

View reviewed changes

Preliminary tvOS support.

b18edd5

sbogolepov force-pushed the tvos_support branch from a47d3c6 to b18edd5 Compare September 7, 2019 05:48

sbogolepov merged commit 237b7ef into master Sep 7, 2019

sbogolepov deleted the tvos_support branch September 7, 2019 05:48

		@@ -0,0 +1,63 @@
		package org.jetbrains.kotlin

		import kotlinx.serialization.Serializable

		@@ -16,6 +16,7 @@

		package org.jetbrains.kotlin.native.interop.gen

		import org.jetbrains.kotlin.konan.target.Architecture

Preliminary tvOS support #3303

Preliminary tvOS support #3303

Conversation

sbogolepov commented Sep 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbogolepov Sep 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbogolepov Sep 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbogolepov Sep 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olonho commented Sep 3, 2019

sbogolepov commented Sep 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbogolepov Sep 3, 2019 •

edited

Loading

sbogolepov Sep 3, 2019 •

edited

Loading

sbogolepov Sep 3, 2019 •

edited

Loading