-
Notifications
You must be signed in to change notification settings - Fork 563
Added embedded bitcode support for ios #1564
Conversation
val bytes = if (marker) { | ||
byteArrayOf() | ||
} else { | ||
val buf = LLVMWriteBitcodeToMemoryBuffer(context.llvmModule) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buf -> buffer (we keep unshortened variable names in K/N).
if (context.shouldContainDebugInfo()) { | ||
DIFinalize(context.debugInfo.builder) | ||
} | ||
} | ||
|
||
private fun embedBitcode(context: Context, marker: Boolean) { | ||
val isIos = context.config.platform.configurables.target.family == Family.IOS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, why IOS is specific here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taken from rust-lang/rust@0e0f74b#diff-a3b24dbe2ea7c1981f9ac79f9745f40aR851 and https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/BackendUtil.cpp#L1237
the section name is just different on that platform. Unsure if this should be on macos as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's Apple specific - so isApple
maybe?
@@ -16,6 +16,7 @@ | |||
|
|||
package org.jetbrains.kotlin.cli.bc | |||
|
|||
import org.jetbrains.kotlin.backend.konan.EmbedBitcode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import seems to be redundant.
@@ -139,5 +140,10 @@ class K2NativeCompilerArguments : CommonCompilerArguments() { | |||
@Argument(value = "--verify_ir", description = "Verify IR") | |||
var verifyIr: Boolean = false | |||
|
|||
@Argument(value = "--embed_bitcode", description = "embed LLVM bitcode in object files") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to enable bitcode embedding by default on macOS and iOS targets.
@olonho WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How much bigger binaries would become then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The smallish sample project I was testing with went from 153MB to 238MB
@@ -130,11 +131,49 @@ internal fun emitLLVM(context: Context) { | |||
irModule.acceptVoid(codegenVisitor) | |||
} | |||
|
|||
when (context.shouldEmbedBitcode()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is bitcode embedding performed before debug info generation intentionally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not
name = "konan.embedded.module", | ||
elemType = int8Type, | ||
elements = bytes.map { Int8(it) }, | ||
isExported = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of passing isExported = true
and then setting linkage to private?
Passing isExported = true
just sets linkage to external (i.e. public).
@@ -176,7 +179,13 @@ internal class LinkStage(val context: Context) { | |||
KonanTarget.MACOS_X64 -> "Versions/A/$dylibName" | |||
else -> error(target) | |||
} | |||
frameworkLinkerArgs = listOf("-install_name", "@rpath/${framework.name}/$dylibRelativePath") | |||
val args = mutableListOf("-install_name", "@rpath/${framework.name}/$dylibRelativePath") | |||
when (context.shouldEmbedBitcode()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the same args be passed to linker when compiling the program instead of framework?
This introduces the flags '--embed_bitcode' and '--embed_bitcode_marker' to make it possible to build bitcode-enabled frameworks.
It seems that bitcode embedding need to be somewhat trickier than that, as we need to ensure, that what's get embedded is actual bitcode after link time optimizations, at least dead code elimination, otherwise code grows awfully large. So we will investigate possibilities here. |
We made some investigations. There are problems (besides mentioned above) with bitcode embedding that aren't solved by this pull request.
So bitcode embedding on watchOS can't be supported by this PR. |
What are the implications when a bitcode enabled app is uploaded to the AppStore, but any include K/N code isn’t actually bitcode enabled? Bitcode is enabled by default for new Xcode projects. |
@dnedrow the implications would be the same as when Xcode project has assembly files. Assembly files are permitted on iOS even when bitcode is enabled. So this solution doesn't seem to cause any observable implications. Xcode is able to recompile an archive with such an application as any other bitcode-enabled application archive. |
Seems this way valid bitcode cannot be generated, so let's close this one for now, and come back again, when focus on that feature again. |
This introduces the flags '--embed_bitcode' and '--embed_bitcode_marker' to
make it possible to build bitcode-enabled frameworks.
Fixes #1202, #1408