Arm32 thumb2 (Cortex-M) support#13815
Conversation
github-actions
Bot
added
cranelift
Subscribe to Label ActionDetailsThis issue or pull request has been labeled: "cranelift", "cranelift:meta", "isle"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
| let settings = SettingGroupBuilder::new("arm32"); | ||
|
|
||
| // ARM32-specific settings can be added here in the future. | ||
| // For now, we start with an empty settings group. |
There was a problem hiding this comment.
cg_clif would at the very least need the thumb2 (use thumb2 rather than the original thumb) and thumb-mode (actually use thumb rather than arm instructions) settings to match LLVM. And the current backend you wrote should error when either is not set.
There was a problem hiding this comment.
I agree about the erroring part, do you have specific names for the flags in mind so that they are more easily compatible with cg_clif?
There was a problem hiding this comment.
I guess other settings are has_fp32, has_fp64, has_dsp, has_cbz, has_it, hwfloat_abi.
There was a problem hiding this comment.
I think you mean these subtarget flags: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/ARM/ARMSubtarget.h and https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/TargetParser/Triple.h - but at least thumb vs arm is selected by the target triple.
There was a problem hiding this comment.
You can select between thumb and arm on a per-function basis in LLVM and Rust, so changing the target triple is not the only way of choosing thumb mode. #[instruction_set(arm::t32)] lowers to what is effectively #[target_feature(enable = "thumb-mode")] in the LLVM backend of rustc. And a couple of rustc targets also use target_features: "+thumb-mode" in the target spec.
|
I re-read all the included code, simplified use statements and removed accidental file edits on other architectures (warning fixes for unused I think this would be a good initial minimal discussion point. I admit that the ABI implementation is more complex than it would need to be for a minimal inclusion (prepared for i64 arguments and results), but also less complex than it will need to become (floating point arguments (hardware float in Sx registers) aren't handled, yet). I can shrink it to the bare minimum if needed. I really would like to do more realistic tests for this new architecture as well. But as far as I understand runtime tests are restricted to the architecture they are built on, qemu-user is not an option, yet. Any recommendations? [I was toying with the idea of rustc_codegen_clif] |
|
As this now forms an atomic step towards arm32 support I mark it as ready for review to start the discussion about its final form. |
cpetig
left a comment
cpetig
left a comment
There was a problem hiding this comment.
The bug described in https://bytecodealliance.zulipchat.com/#narrow/channel/217117-cranelift/topic/Arm32.20backend.20.28for.20now.20thumb-2.20only.29.20started/near/608325866 is likely caused by this.
| // Save FP and LR onto the stack. | ||
| // push {r11, lr} — equivalent to STMDB.W sp!, {sp-reg..lr-reg}. | ||
| // Register list: r11 = bit 11, lr = bit 14 → mask = (1<<11) | (1<<14). | ||
| let fp_lr_mask: u16 = (1 << 11) | (1 << 14); |
There was a problem hiding this comment.
I think this should save clobbers as well.
| // Pop FP and LR. | ||
| // pop {r11, lr} — LDMIA.W sp!, {list}. | ||
| // Register list: r11 = bit 11, lr = bit 14 → mask same as push. | ||
| let fp_lr_mask: u16 = (1 << 11) | (1 << 14); |
| mask |= 1 << r.to_reg().hw_enc(); | ||
| } | ||
| // Add bit 15 (PC) so that popping into the return address triggers a return. | ||
| mask |= 1 << 15; |
There was a problem hiding this comment.
This is a nice optimization but doesn't combine with frame setup.
|
Moving review to @cfallin because (a) I'm not really knowledgeable about arm32 and (b) I will be out of office next week and don't have time to look at it this week. |
|
(I'm also on PTO this week, and traveling next week, so I will look at this the week of Mon Jul 20; happy to do so of course) |
4 participants
This is related to #1173 (might fix parts of that issue once complete)
The goal is to implement ARM32 thumb2 not the original ARM32 commands of e.g. ARM9 as they have less practical relevance.
This for now is a skeleton to get a newly added arm32 backend compiling. Further work will gradually implement instructions and functionality.
Of course the naming and scope of the architecture can already be discussed here.