Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cranelift: ARM32 backend: active maintenance, or remove? #3721

Closed
cfallin opened this issue Jan 24, 2022 · 7 comments · Fixed by #3799
Closed

Cranelift: ARM32 backend: active maintenance, or remove? #3721

cfallin opened this issue Jan 24, 2022 · 7 comments · Fixed by #3799
Labels
cranelift:discussion cranelift Issues related to the Cranelift code generator

Comments

@cfallin
Copy link
Member

cfallin commented Jan 24, 2022

In the Cranelift biweekly today, we discussed the ARM32 backend and how to handle it with respect to our ISLE transition, and maintenance / breaking changes in internal APIs in general.

The current state of the ARM32 backend is incomplete: it supports 32-bit code, but panics on any 64-bit operation, so it does not yet support (for example) Wasm-MVP with the cranelift-wasm frontend, nor would it support most code generated by cg_clif. The intent when merging the partial backend was to allow it to mature in-tree with further contributions. However, that hasn't materialized in the year or so since merging.

We have made changes as needed to keep it compiling, but there is a larger question of what happens to an incomplete backend with no active maintainers or users, especially if it implies more significant amounts of work. With our ISLE DSL transition, we would need to invest nontrivial time to move the backend over; and the upcoming regalloc2 transition would require more effort as well.

It seems reasonable to ask: if the backend is incomplete, and no one is using or maintaining it, should we remove it instead? Consensus in our meeting today was that this seems reasonable. However, if anyone would like to work toward a usable and maintained ARM32 backend, now is the time to step forward!

Thoughts?

@akirilov-arm akirilov-arm added cranelift Issues related to the Cranelift code generator cranelift:discussion labels Jan 26, 2022
@posborne
Copy link

Hi @cfallin, I understand the situation you and the maintainers are in. We are not currently a user of wasmtime/cranelift on arm32 but we have been evaluating projects to potentially target wasm via wasmtime on our fleet of IoT hubs. To consider use of wasmtime/cranelift, arm32 support is a must as we continue to support software updates for a very large number of products using arm32 (in addition to products based on aarch64/x86_64/etc.) and would guess that this is true for other potential users in this space. Thus far, one of the reasons we have not yet built solutions around wasm is the lack of arm32 support which we had hoped would be something that would get love from ARM or others.

I will reach out to some of my contacts within the broader Samsung org to see if we are currently in a position to provide any assistance; I do feel that if IoT is in fact an area where we would like wasmtime/cranelift to be applicable along with the goal of providing a portable bytecode that arm32 support is a critical feature.

@cfallin
Copy link
Member Author

cfallin commented Jan 27, 2022

@posborne -- hello and thanks for helping out here!

For context, I think it would probably take 1-2 months of full-time work by a compiler engineer to bring the current incomplete arm32 backend up to a working state where it supports Wasm, and would need ongoing engagement by someone who would address bugs, keep the backend up-to-date wrt other refactors, etc.

We also are reaching a point where we'll need to do something soon, as we have ongoing efforts to update the architecture-independent bits of the compiler (e.g. the register allocator) and an unmaintained backend becomes a blocker for those. So, if someone were to jump in and adopt the arm32 backend, it would need to be somewhat soon.

I don't doubt the value of arm32 at all, and all else being equal, would love to have full and complete support for it! It's really just a matter of resources. And, it's worth mentioning that if we do end up removing it from the tree now, someone would always be welcome to step in, grab the old source from git history as a starting point, and bring it up to date, then contribute it, as long as there is a reasonable ongoing-maintenance story.

@posborne
Copy link

@cfallin Thank you for the context; I think we're on the same page and, unfortunately, don't think we'll have the resources available to help in the near term so I think the plan to remove the half-baked arm32 for now makes sense (though I have still reached out to confirm). I just wanted to make sure we expressed our interest in targeting arm32 but also appreciate the need to move the project forward.

@sparker-arm
Copy link
Contributor

A smaller issue, but isn't cranelift's ABI also currently hard-coded for 64-bit architectures only?

Either way, an unmaintained (and unusable?) backend seems like quite a burden for a project that wants to change.

@akirilov-arm
Copy link
Contributor

@posborne Is your IoT use case based on M profile cores (e.g. Cortex-M4) or A profile ones (such as Cortex-A53)?

@posborne
Copy link

posborne commented Feb 7, 2022

@posborne Is your IoT use case based on M profile cores (e.g. Cortex-M4) or A profile ones (such as Cortex-A53)?

We target a variety of application processors running embedded Linux in addition to targets running Android or Tizen. The two targets with greatest volume in production are the NXP IMX6UL and IMX6ULL which are both Cortex-A7 (we also target several aarch64 based processors). We don't currently have any integrations on MIPS or RISC-V but both of those have come up as possibilities at different points in time.

@akirilov-arm
Copy link
Contributor

OK, so definitely the A profile.

The reason I am asking is that while it is feasible to add support for the M profile to Cranelift (since it is mostly just a code generator), it would be a challenge to do the same to Wasmtime because the latter depends on a full-featured operating system like Linux (that provides mechanisms such as mmap()); the M profile processors have difficulties supporting environments of that kind (given that they lack memory management units, among other limitations). This is somewhat similar to supporting #[no_std].

cfallin added a commit to cfallin/wasmtime that referenced this issue Feb 14, 2022
In bytecodealliance#3721, we have been discussing what to do about the ARM32 backend in
Cranelift. Currently, this backend supports only 32-bit types, which is
insufficient for full Wasm-MVP; it's missing other critical bits, like
floating-point support; and it has only ever been exercised, AFAIK, via
the filetests for the individual CLIF instructions that are implemented.

We were very very thankful for the original contribution of this
backend, even in its partial state, and we had hoped at the time that we
could eventually mature it in-tree until it supported e.g. Wasm and
other use-cases. But that hasn't yet happened -- to the blame of no-one,
to be clear, we just haven't had a contributor with sufficient time.

Unfortunately, the existence of the backend and lack of active
maintainer now potentially pose a bit of a burden as we hope to make
continuing changes to the backend framework. For example, the ISLE
migration, and the use of regalloc2 that it will allow, would need all
of the existing lowering patterns in the hand-written ARM32 backend to
be rewritten as ISLE rules.

Given that we don't currently have the resources to do this, we think
it's probably best if we, sadly, for now remove this partial backend.
This is not in any way a statement of what we might accept in the
future, though. If, in the future, an ARM32 backend updated to our
latest codebase with an active maintainer were to appear, we'd be happy
to merge it (and likewise for any other architecture!). But for now,
this is probably the best path. Thanks again to the original contributor
@jmkrauz and we hope that this work can eventually be brought back and
reused if someone has the time to do so!
cfallin added a commit to cfallin/wasmtime that referenced this issue Feb 14, 2022
In bytecodealliance#3721, we have been discussing what to do about the ARM32 backend in
Cranelift. Currently, this backend supports only 32-bit types, which is
insufficient for full Wasm-MVP; it's missing other critical bits, like
floating-point support; and it has only ever been exercised, AFAIK, via
the filetests for the individual CLIF instructions that are implemented.

We were very very thankful for the original contribution of this
backend, even in its partial state, and we had hoped at the time that we
could eventually mature it in-tree until it supported e.g. Wasm and
other use-cases. But that hasn't yet happened -- to the blame of no-one,
to be clear, we just haven't had a contributor with sufficient time.

Unfortunately, the existence of the backend and lack of active
maintainer now potentially pose a bit of a burden as we hope to make
continuing changes to the backend framework. For example, the ISLE
migration, and the use of regalloc2 that it will allow, would need all
of the existing lowering patterns in the hand-written ARM32 backend to
be rewritten as ISLE rules.

Given that we don't currently have the resources to do this, we think
it's probably best if we, sadly, for now remove this partial backend.
This is not in any way a statement of what we might accept in the
future, though. If, in the future, an ARM32 backend updated to our
latest codebase with an active maintainer were to appear, we'd be happy
to merge it (and likewise for any other architecture!). But for now,
this is probably the best path. Thanks again to the original contributor
@jmkrauz and we hope that this work can eventually be brought back and
reused if someone has the time to do so!
cfallin added a commit that referenced this issue Feb 14, 2022
In #3721, we have been discussing what to do about the ARM32 backend in
Cranelift. Currently, this backend supports only 32-bit types, which is
insufficient for full Wasm-MVP; it's missing other critical bits, like
floating-point support; and it has only ever been exercised, AFAIK, via
the filetests for the individual CLIF instructions that are implemented.

We were very very thankful for the original contribution of this
backend, even in its partial state, and we had hoped at the time that we
could eventually mature it in-tree until it supported e.g. Wasm and
other use-cases. But that hasn't yet happened -- to the blame of no-one,
to be clear, we just haven't had a contributor with sufficient time.

Unfortunately, the existence of the backend and lack of active
maintainer now potentially pose a bit of a burden as we hope to make
continuing changes to the backend framework. For example, the ISLE
migration, and the use of regalloc2 that it will allow, would need all
of the existing lowering patterns in the hand-written ARM32 backend to
be rewritten as ISLE rules.

Given that we don't currently have the resources to do this, we think
it's probably best if we, sadly, for now remove this partial backend.
This is not in any way a statement of what we might accept in the
future, though. If, in the future, an ARM32 backend updated to our
latest codebase with an active maintainer were to appear, we'd be happy
to merge it (and likewise for any other architecture!). But for now,
this is probably the best path. Thanks again to the original contributor
@jmkrauz and we hope that this work can eventually be brought back and
reused if someone has the time to do so!
mpardesh pushed a commit to avanhatt/wasmtime that referenced this issue Mar 17, 2022
…ance#3799)

In bytecodealliance#3721, we have been discussing what to do about the ARM32 backend in
Cranelift. Currently, this backend supports only 32-bit types, which is
insufficient for full Wasm-MVP; it's missing other critical bits, like
floating-point support; and it has only ever been exercised, AFAIK, via
the filetests for the individual CLIF instructions that are implemented.

We were very very thankful for the original contribution of this
backend, even in its partial state, and we had hoped at the time that we
could eventually mature it in-tree until it supported e.g. Wasm and
other use-cases. But that hasn't yet happened -- to the blame of no-one,
to be clear, we just haven't had a contributor with sufficient time.

Unfortunately, the existence of the backend and lack of active
maintainer now potentially pose a bit of a burden as we hope to make
continuing changes to the backend framework. For example, the ISLE
migration, and the use of regalloc2 that it will allow, would need all
of the existing lowering patterns in the hand-written ARM32 backend to
be rewritten as ISLE rules.

Given that we don't currently have the resources to do this, we think
it's probably best if we, sadly, for now remove this partial backend.
This is not in any way a statement of what we might accept in the
future, though. If, in the future, an ARM32 backend updated to our
latest codebase with an active maintainer were to appear, we'd be happy
to merge it (and likewise for any other architecture!). But for now,
this is probably the best path. Thanks again to the original contributor
@jmkrauz and we hope that this work can eventually be brought back and
reused if someone has the time to do so!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:discussion cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants