Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Document RFC 2867: instruction_set attribute #1253
Document RFC 2867: instruction_set attribute #1253
Changes from 2 commits
8fda26b
ef70365
e389abd
8919fc1
3315bbc
33e3271
d27f43f
dbb8e2a
c47e316
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "
ARMv4T
devices", might it perhaps be a little more specific to say "theARMv4T
architecture"? I know there likely won't be any confusion, but I'd like to use precise phrasing when possible.Part of the problem talking about this clearly is that the reference has shied away from platform specific stuff, so it can be difficult to talk about things without also talking about target triples and such. AFAIK, the presence of
ARMv4T
can't be detected throughcfg
can it?Also, can this maybe be structured so that it can be easily be extended in the future, and makes it clear which values to use? Perhaps something like:
The following values are available on targets for the
ARMv4T
architecture:arm::a32
— Uses ARM code.arm::t32
— Uses Thumb code.Or something like that? Then, if support for other architectures is added, they can just add more lists (or if more ARM modes are added, just extend the list).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use the armv4t target triple to detect it afaik, @Lokathor is the best person to ask as he's the most prevalent user currently
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So first minor point: ARMv? (with a
v
) and ARM? (without av
) are different numbering systems, because of course. So, if you go looking up details about this stuff keep that in mind.Starting with ARMv4T and onward, ARM CPUs support a system called "interworking". This is currently what the
instruction_set
attribute is all about. I'm told that theoretically MIPS also has some sort of system where theinstruction_set
attribute would be appropriate, but I was told that once ever in passing so we can reasonably treat this attribute at ARM only for now.Interworking is available in most ARM architectures that came after ARMv4T, but not all of them.
I had to go double check on wikipedia:
Looking at Rust's target list,
thumbv6m-none-eabi
is described as "Bare Cortex-M0, M0+, M1", so I have to assume that you should not use theinstruction_set
attribute on that target to set a function fora32
code. However, it's still logically consistent to set a function fort32
on a Cortex-M. That's already the default, and you'd basically be just more strongly re-stating that.In terms of what you can check with
cfg
at compile time, beyondtarget_arch="arm"
, there's alsotarget_feature = "thumb-mode"
which at least tells you the default code mode.As far as I know, there isn't a specific way to check for the presence of interworking. If I ask rustc what it thinks about the
thumbv4t-none-eabi
target:Which... sure doesn't seem to indicate if interworking is specifically allowed.
I know that the assembler knows if it's assembling interworking code (example, but I don't know if rustc lets you know this in any way.
But if we go check the
thumbv6m-none-eabi
target we see:So we can probably say that if
target_feature="mclass"
is set then you should not try interworking.@xd009642 how does the
rustc
decide if the attribute is allowed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking back at code to refresh myself, there's a
has_thumb_interworking
bool for each target, it's only true for:So docs should be adjusted in some way to mention armv5te as well (I'll do that tomorrow it's after midnight here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uhhhhhhhhhhhhhhhhhh, I've certainly used the attribute on the
thumbv4t-none-eabi
target, so either your check is broken somehow or you're looking at the wrong part of the code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope I just missed it, each spec is in their own file and I stopped reading when I hit the module mod in commit diff, should have read
thumbv4t_none_eabi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there's a way to query the info in each spec i.e. https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/spec/armv4t_none_eabi.rs we could amend the docs to instead tell people how to see if their target supports interworking given
has_thumb_interworking: true,
is a requirement for the attributeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
asm_args
is entirely unused.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh maybe that's why my code breaks randomly sometimes