Add tool to create multi-arch iOS gen_snapshot #4948
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds create_macos_gen_snapshot.py, which can be used to generate a multi-architecture (x86_64, i386) gen_snapshot fat binary. The resulting binary can then be run in the desired mode using: /usr/bin/arch -i386 path/to/gen_snapshot /usr/bin/arch -x86_64 path/to/gen_snapshot When creating AOT snapshots for iOS, running as an i386 binary will generate armv7 code, whereas running as an x86_64 binary will generate arm64 code. The primary user of this script is the build bot.
chinmaygarde
approved these changes
Apr 7, 2018
|
Does Otherwise, I really like this trick. Simplifies all the various asset variants and where one would go about looking for them. |
|
Currently it doesn't have such a flag. /cc @rmacnak-google @a-siva how feasible would it be to enable gen_snapshot to take target architecture as a flag so we could just ship a single binary? |
|
Oh, I didn't mean a single architecture |
cbracken
added a commit
to cbracken/flutter
that referenced
this pull request
Apr 17, 2024
When performing artifact lookups for Artifact.genSnapshot, a TargetPlatform is used to determine the name of the tool, typically `gen_snapshot_$TARGET_ARCH`. Formerly, this tool was always named `gen_snapshot`. The astute reader may ask "but Chris, didn't we support TWO target architectures on iOS and therefore need TWO gen_snapshot binaries?" Yes, we did support both armv7 and arm64 target architectures on iOS. No, we didn't have two gen_snapshot binaries. At the time, the bitness of the gen_snapshot tool needed to match the bitness of the target architecture. As such we did *build* two gen_snapshots: * A 32-bit x86 binary that emitted armv7 AOT code * A 64-bit x64 binary that emitted arm64 AOT code However, to avoid having to do a lot of work plumbing through suffixed gen_snapshot names, the author of that work elected to "cleverly" lipo the two together into a single multi-architecture macOS binary. See: flutter/engine#4948 This was later remediated over the course of several patches, including: See: flutter#37445 See: flutter/engine#10430 See: flutter/engine#22818 However, there were still cases (notably --local-engine workflows in the tool) where we weren't computing the target platform and thus referenced the generic gen_snapshot tool. See: flutter#38933 Fixed in: flutter/engine#28345 The test removed in this PR, which ensured that null SnapshotType.platform was supported was introduced in flutter#11924 as a followup to flutter#11820 when the snapshotting logic was originally extracted to the `GenSnapshot` class. Since there are no longer any cases where TargetPlatform isn't passed when looking up Artifacts.genSnapshot, we can safely make the platform non-nullable and remove the test. This is pre-factoring towrards the removal of the generic gen_snapshot artifact, which is pre-factoring towards the goal of building gen_snapshot binaries with an arm64 host architecture, and eliminate the need to use Rosetta during iOS and macOS Flutter builds. Issue: flutter#101138 Issue: flutter#103386
9 tasks
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Apr 18, 2024
When performing artifact lookups for `Artifact.genSnapshot` for macOS desktop builds, a `TargetPlatform` is used to determine the name of the tool, typically `gen_snapshot_$TARGET_ARCH`. Formerly, this tool was always named `gen_snapshot`. The astute reader may ask "but Chris, didn't we support TWO target architectures on iOS and therefore need TWO `gen_snapshot` binaries?" Yes, we did support both armv7 and arm64 target architectures on iOS. But no, we didn't initially have two `gen_snapshot` binaries. We did *build* two `gen_snapshots`: * A 32-bit x86 binary that emitted armv7 AOT code * A 64-bit x64 binary that emitted arm64 AOT code At the time, the bitness of the `gen_snapshot` tool needed to match the bitness of the target architecture, and to avoid having to do a lot of work plumbing through suffixed `gen_snapshot` names, the author of that work (who, as evidenced by this patch, is still paying for his code crimes) elected to "cleverly" lipo the two together into a single multi-architecture macOS binary still named `gen_snapshot`. See: flutter/engine#4948 This was later remediated over the course of several patches, including: * flutter/engine#10430 * flutter/engine#22818 * #37445 However, there were still cases (notably `--local-engine` workflows in the tool) where we weren't computing the target platform and thus referenced the generic `gen_snapshot` tool. See: #38933 Fixed in: flutter/engine#28345 The test removed in this PR, which ensured that null `SnapshotType.platform` was supported was introduced in #11924 as a followup to #11820 when the snapshotting logic was originally extracted to the `GenSnapshot` class, and most invocations still passed a null target platform. Since there are no longer any cases where `TargetPlatform` isn't passed when looking up `Artifact.genSnapshot`, we can safely make the platform non-nullable and remove the test. This is pre-factoring towards the removal of the generic `gen_snapshot` artifact from the macOS host binaries (which are currently unused since we never pass a null `TargetPlatform`), which is pre-factoring towards the goal of building `gen_snapshot` binaries with an arm64 host architecture, and eliminate the need to use Rosetta during iOS and macOS Flutter builds. Part of: #101138 Umbrella issue: #103386 Umbrella issue: #69157 No new tests since the behaviour is enforced by the compiler.
gilnobrega
pushed a commit
to gilnobrega/flutter
that referenced
this pull request
Apr 22, 2024
When performing artifact lookups for `Artifact.genSnapshot` for macOS desktop builds, a `TargetPlatform` is used to determine the name of the tool, typically `gen_snapshot_$TARGET_ARCH`. Formerly, this tool was always named `gen_snapshot`. The astute reader may ask "but Chris, didn't we support TWO target architectures on iOS and therefore need TWO `gen_snapshot` binaries?" Yes, we did support both armv7 and arm64 target architectures on iOS. But no, we didn't initially have two `gen_snapshot` binaries. We did *build* two `gen_snapshots`: * A 32-bit x86 binary that emitted armv7 AOT code * A 64-bit x64 binary that emitted arm64 AOT code At the time, the bitness of the `gen_snapshot` tool needed to match the bitness of the target architecture, and to avoid having to do a lot of work plumbing through suffixed `gen_snapshot` names, the author of that work (who, as evidenced by this patch, is still paying for his code crimes) elected to "cleverly" lipo the two together into a single multi-architecture macOS binary still named `gen_snapshot`. See: flutter/engine#4948 This was later remediated over the course of several patches, including: * flutter/engine#10430 * flutter/engine#22818 * flutter#37445 However, there were still cases (notably `--local-engine` workflows in the tool) where we weren't computing the target platform and thus referenced the generic `gen_snapshot` tool. See: flutter#38933 Fixed in: flutter/engine#28345 The test removed in this PR, which ensured that null `SnapshotType.platform` was supported was introduced in flutter#11924 as a followup to flutter#11820 when the snapshotting logic was originally extracted to the `GenSnapshot` class, and most invocations still passed a null target platform. Since there are no longer any cases where `TargetPlatform` isn't passed when looking up `Artifact.genSnapshot`, we can safely make the platform non-nullable and remove the test. This is pre-factoring towards the removal of the generic `gen_snapshot` artifact from the macOS host binaries (which are currently unused since we never pass a null `TargetPlatform`), which is pre-factoring towards the goal of building `gen_snapshot` binaries with an arm64 host architecture, and eliminate the need to use Rosetta during iOS and macOS Flutter builds. Part of: flutter#101138 Umbrella issue: flutter#103386 Umbrella issue: flutter#69157 No new tests since the behaviour is enforced by the compiler.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds create_macos_gen_snapshot.py, which can be used to generate a
multi-architecture (x86_64, i386) gen_snapshot fat binary. The resulting
binary can then be run in the desired mode using:
/usr/bin/arch -i386 path/to/gen_snapshot
/usr/bin/arch -x86_64 path/to/gen_snapshot
When creating AOT snapshots for iOS, running as an i386 binary will
generate armv7 code, whereas running as an x86_64 binary will generate
arm64 code.
The primary user of this script is the build bot.