Remove the reg_thumb register class for asm! on ARM #90796
Conversation
|
Some changes occured to rustc_codegen_gcc cc @antoyo |
rust-highfive
added
the
S-waiting-on-review
Amanieu
force-pushed
the
remove_reg_thumb
branch
from
0c52d55
to
9dc75c8
Compare
This comment has been minimized.
This comment has been minimized.
apiraino
added
the
T-compiler
Amanieu
force-pushed
the
remove_reg_thumb
branch
from
9dc75c8
to
8548079
Compare
|
I'm not totally familiar with thumb2, but i believe that you explained that when a low register is used in thumb2 code it's less total bytes to encode the code than when a high register is selected. If so, that seems a perfectly fine reason for a user to want to use the reg_thumb class, and we should keep it. |
|
It seems like something that could be added later if there is demand for it. At the moment I'm not too happy about the confusion between |
|
well, since But since you're the one who will have to put it back in later, I won't get too worked up about you taking it out temporarily until someone specifically needs it. |
|
ping @joshtriplett |
|
Taking this out temporarily for stabilization seems reasonable, and we can consider adding a feature-gated version of this back in later. @bors r+ |
|
📌 Commit 85480793a42084a6e45681ba305330c4fe20dd54 has been approved by |
bors
added
S-waiting-on-bors
|
☔ The latest upstream changes (presumably #91224) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
Also restricts r8-r14 from being used on Thumb1 targets as per rust-lang#90736.
Amanieu
force-pushed
the
remove_reg_thumb
branch
from
8548079
to
908f300
Compare
|
@bors r=joshtriplett |
|
📌 Commit 908f300 has been approved by |
bors
added
S-waiting-on-bors
…plett Remove the reg_thumb register class for asm! on ARM Also restricts r8-r14 from being used on Thumb1 targets as per rust-lang#90736. cc `@Lokathor` r? `@joshtriplett`
…askrgr Rollup of 6 pull requests Successful merges: - rust-lang#87599 (Implement concat_bytes!) - rust-lang#89999 (Update std::env::temp_dir to use GetTempPath2 on Windows when available.) - rust-lang#90796 (Remove the reg_thumb register class for asm! on ARM) - rust-lang#91042 (Use Vec extend instead of repeated pushes on several places) - rust-lang#91634 (Do not attempt to suggest help for overly malformed struct/function call) - rust-lang#91685 (Install llvm tools to sysroot when assembling local toolchain) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…plett Remove the reg_thumb register class for asm! on ARM Also restricts r8-r14 from being used on Thumb1 targets as per rust-lang#90736. cc ``@Lokathor`` r? ``@joshtriplett``
| r5: reg = ["r5", "v2"], | ||
| r7: reg = ["r7", "v4"] % frame_pointer_r7, | ||
| r8: reg = ["r8", "v5"] % not_thumb1, | ||
| r10: reg = ["r10", "sl"] % not_thumb1, | ||
| r11: reg = ["r11", "fp"] % frame_pointer_r11, |
There was a problem hiding this comment.
because this line does not add not_thumb1, thumb1 inline asm can still specify r11 as a clobber, which seems at odds with the documentation
|
Cortex-m0 uses the thumbv6-m instruction set, which contains some thumb2 instructions but not the full set of 32 bit instructions that are part of thumb2. Thumbv6-m does not pass the has_feature("thumb2") test, presumably because it does not implement the full thumb2 set of instructions. However, that means that this PR prevents clobbering r8 + on cortex-m0, despite the fact that those registers exist and can be used for several instructions. Is this a bug? Am I misunderstanding something? |
|
You're right, we should allow these registers to be clobbered even if they cannot be used as operands. Also nice catch about r11, that was not intended. I will prepare a PR which fixes these issues. |
|
Thank you! |
Also restricts r8-r14 from being used on Thumb1 targets as per #90736.
cc @Lokathor
r? @joshtriplett