dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.09k stars 1.56k forks source link

Allow gen_snapshot to generate instructions snapshots for multiple architectures. #31709

Open chinmaygarde opened 6 years ago

chinmaygarde commented 6 years ago

Today, the x86_64 host gen_snapshot can only generate the AOT instructions snapshots for aarch64 and x86_64 (with a different version of gen_snapshot for each target). Similarly, an i386 gen_shapshot is required for arm-v7a instructions. This requires the tooling the ship (and the users to download) a target specific version of gen_snapshot. It also adds a lot of complicated logic in the tooling to select the gen_snapshot for each host-target pair.

It would be great if a single gen_shapshot were able to generate instructions for multiple target architectures. Fewer variants of the same target would need to be build, tested, uploaded to the cloud and downloaded by our users.

chinmaygarde commented 6 years ago

CC @a-siva

a-siva commented 6 years ago

cc @rmacnak-google @zanderso

zanderso commented 6 years ago

Since this would entail a significant refactoring of the VM, to help prioritize this, it would be good to have an estimate of what the percentage savings would be relative to all the other bytes that a Flutter end-user has to pull down.

mraleph commented 6 years ago

This would be rather easy to do after the refactoring planed for the compilation pipeline where we aim to separate compiler from the rest of the VM. Dart 2 is currently taking priority but I would expect us to be able to get back to it closer to the end of Q1.

keertip commented 5 years ago

@mraleph, now that Dart 2 is out and well, is this something that can be worked on. We are starting to look at building gen_snapshot internally, and it would be so much easier to build just one flavor of the binary.

mraleph commented 5 years ago

I am working on enabling generating arm32 snapshots with 64-bit gen_snapshot binary this quarter.

I think this would lay a lot of the ground work needed for having just a single gen_snapshot - but whether we can implement this or not depends on how much the first step takes.

mehmetf commented 5 years ago

This just became high priority for Google.

We can't build Flutter engine in Google without this change because the internal toolchain no longer allows building x86 binaries.

mehmetf commented 5 years ago

Just to be clear, the ability to generate arm32 snapshots with 64-bit gen_snapshot is the high priority task. We can live with the fact that android vs ios requires different binaries.

mehmetf commented 5 years ago

What happens today when I try to build gen_snapshot and pass

    "TARGET_ARCH_ARM",
    "TARGET_OS_ANDROID",

as defines.


runtime/platform/globals.h:310:2: error: Mismatched Host/Target architectures.
#error Mismatched Host/Target architectures.
 ^
1 error generated.
mraleph commented 5 years ago

I have started working on this - but this is still at least a month away, if we want to do it the right way.

eseidelGoogle commented 5 years ago

@mraleph I spoke with @mehmetf just now just to double-check that this was the right thing for us to spend on. Between he and I we imagined a work-around by which we built the 32-bit binaries in another manner outside of the internal toolchain. The engineering to make that work would be non-zero (and a bit of a hack) so if you spending on this is good for the VM, we should do it. But if you feel this is wasted work to invest in we could look more deeply into the hacks mehemet and I discussed (which would be specific to the internal systems).

eseidelGoogle commented 5 years ago

@cbracken also reminds me that this would benefit mac as well, where Mojave now warns for 32-bit binaries (which gen_snapshot for older iOS phones is) and whatever is after Mojave would disallow running 32-bit binaries supposedly.

mehmetf commented 5 years ago

There are a couple of projects in Google that are asking for this. I see PRs fly by. It would be great if we have an ETA. Is end of Q2 a reasonable estimate?

dnfield commented 5 years ago

Apple announced at the last WWDC that Mojave would be the last macOS to support 32 bit. This could very well mean that we only have until September before this becomes a more serious problem.

mraleph commented 5 years ago

I had discussions with @mkustermann about this issue and we arrived to the conclusion that our original approach would probably require to continue rather tedious refactoring for another quarter. It has various benefits (e.g. we essentially audit every single line in the compiler that interacts with runtime system and make sure that word size discrepancies are discovered) and we will also arrive to the setup in which a single gen_snapshot could be made to generate code for multiple target architectures.

However this would require way too much effort.

Instead we are going to try something much simpler, where we don't decouple runtime and compiler entirely and instead just audit backend for occurances of known constants and methods that depend on word size.

This should allow us to get to X64 gen_snapshot which can generate ARM32 code much faster.

I have started on this and it indeed seems that the progress could be much faster in this setup.

I will update this issue next week with the results.

cbracken commented 5 years ago

@mraleph any updates on this?

mraleph commented 5 years ago

@cbracken update is at https://github.com/flutter/flutter/issues/22598#issuecomment-487544359, we are not pursuing fixing this bug anymore, we will just enable building 64-bit gen_snapshot that can generated ARM32 code. This bug is much more generic.

tldr version of the update: I have successfully built such gen_snapshot, it now passes all the tests and now I am breaking down and upstreaming the changes from the ugly CL that I made.