Add support for riscv64 desktop linux engine - #178712

@vhaudiquet
This patch set is required for a proper risc-v 32/64 engine build:
https://github.com/meta-flutter/workspace-automation/tree/v2.0/patches/flutter-engine

Also found here:
https://github.com/meta-flutter/meta-flutter/tree/master/recipes-graphics/flutter-engine/files

They’ve been around since August of last year.

Two key issues:

• engine needs to update llvm toolchain (cipd)
• third_party deps need updates (before this repo)

committing flutter changes without the above two is a bit of a hack.

I've only had to apply a patch to third_party swiftshader (as explained above). Other patches seem to have already been applied, or to not be necessary anymore?

I did not need anything except the swiftshader patch and this PR to build the engine.

Did the llvm cipd get updated recently? If so in what version. Patches referenced are for stable (3.57.7) + llvm 21.

I would think so, as effectively I tried applying my PR to the latest stable and it failed to build, so that one needed more patches.
It might be related to the recently merged PRs that provide a Flutter engine for riscv64 Android.

@robert-ancell This allows cross-compiling the engine on an x86_64 host for a riscv64 target, so you don't technically need a riscv64 builder to do that. But to run that engine, you are right that you need a riscv64 host; for which I think all of the community understands that the support is experimental. But maybe we can still add a message somewhere?

I see now that the "Google testing" failed. The test page says

Internal tests failed. Please contact a Google developer to help investigate the failure.

Can somebody help with that? 😃

Ping @jtmcdole on landing and testing this.

Hello @jtmcdole, happy new year! 😃
Any updates? Is there anything still misssing?

Back from holidays, let me take a look

Now that the tool PR has landed, is there a timeline for this one?

So right now, lets look at landing this; then get a patch in; then consider a x64 builder that can make artifacts at head.

If I can help getting the patch in for third_party/swiftshader, tell me 😃
I didn't know it was possible to patch this directly for Flutter usage. Also, I can help testing resulting artifacts, and testing everything basically, on RISC-V hardware. Let me know

Now that the tool PR has landed, is there a timeline for this one?

So right now, lets look at landing this; then get a patch in; then consider a x64 builder that can make artifacts at head.

If I can help getting the patch in for third_party/swiftshader, tell me 😃 I didn't know it was possible to patch this directly for Flutter usage. Also, I can help testing resulting artifacts, and testing everything basically, on RISC-V hardware. Let me know

The PR description says "For now, building the engine on RISC-V still requires another patch:" - since we'd be building this on x86, are we blocked waiting on this patch?

@jason-simmons - can you help with how we would maintain a flutter-specific patch for:

diff --git a/src/Reactor/BUILD.gn b/src/Reactor/BUILD.gn
index 67dfeb0ec..dcb13614a 100644
--- a/src/Reactor/BUILD.gn
+++ b/src/Reactor/BUILD.gn
@@ -307,7 +307,7 @@ if (supports_subzero) {

 if (supports_llvm) {
   swiftshader_source_set("swiftshader_llvm_reactor") {
-    llvm_dir = "../../third_party/llvm-10.0"
+    llvm_dir = "../../third_party/llvm-16.0"

     deps = [
       ":swiftshader_reactor_base",

The SwiftShader upstream tried to land that patch 2 years ago. But apparently it was reverted due to an incompatibility with Chromium.
(see https://swiftshader.googlesource.com/SwiftShader/+/eb75201a4e0354a36d315dd01077092ec9aa2356)

If we want to use that patch in Flutter, then that would involve creating a mirror of SwiftShader on https://flutter.googlesource.com/ and adding the patch to a branch.

The PR description says "For now, building the engine on RISC-V still requires another patch:" - since we'd be building this on x86, are we blocked waiting on this patch?

Sorry, I meant for RISC-V. We will need that patch if we want to merge and build at the same time.

i've created a mirror. I'll see about landing the patch there and then updating DEPs.

Hey @jtmcdole this hasn't been updated in a while but is approved, are we still sitting on this?

Hey @jtmcdole this hasn't been updated in a while but is approved, are we still sitting on this?

The idea was to merge that and also start to build the engine for riscv64 at the same time.
Unfortunately, building the engine still requires a patch to third_party/swiftshader, so @jtmcdole opened #181225 to add that patch.

The issue is that #181225 causes another problem, as it breaks builds on different cases, I'm not sure where (multiple tests are failing in CI, I think there are real failures on Windows and maybe others). I tried to reproduce locally but even with the swiftshader patch of #181225 the builds I tried did work 😃

I think there was a lack of time to debug that further, which is why the PR is still in this dangling state.

still waiting on the patch from @jtmcdole, I've pinged him on that pr

@jtmcdole friendly ping

third_party/swiftshader doesn't build for me on infra. I've not had time to look into what is missing to get that building from the mirror. I can try an update the mirror / patch set sometime this week.

I'm taking a quick look this morning.

I'm taking a quick look this morning.

FYI I'll be splitting this into 3 PRs, one per commit. I was told that it made more sense scope-wise

So:

• I rebased this onto upstream/master. This might have been a mistake as it dropped CICD label 😢
• I applied latest suggestions from @jtmcdole (thanks!)
• I have removed other changes from this PR. In order to be able to build, it needs
  • Patch atkversion.h in sysroots to add C linkage declarations #195883
  • Another commit, vhaudiquet@7a1010d, that I cannot send as a PR given that I have a limit of 2 PRs opened at a time. I'll send this when I can, taking in latest review from @jtmcdole as well.

All of this is needed for building. Reminder that this allows cross-building the engine, so technically build-test does not need riscv64 hardware, as this builds amd64 -> riscv64. However you do need RISC-V hardware in order to test that built engine afterwards.

I produce regular builds of the engine for riscv64, all hosted on: https://flutter-experimental.ubuntu.com
Those builds are production-tested in the Ubuntu Installer present in the Ubuntu Desktop images for RISC-V. So I would say that I am now a lot more confident in this being ready then I was when I first sent that PR.

I will build-test locally again with all the 3 commits rebased and updated, I'll follow up with the results here.

autosubmit label was removed for flutter/flutter/178712, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

• Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a member of flutter-hackers before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

FYI I retested locally

et build -c ci/linux_debug_riscv64
et build -c ci/linux_profile_riscv64
et build -c ci/linux_release_riscv64

All 3 are build ok with the 3 commits mentioned above.