-
Notifications
You must be signed in to change notification settings - Fork 163
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
[RFC] Defining an ELF attribute for RISC-V target profiles #409
Conversation
This commit defines an ELF attribute which allows tools to set a RISC-V target profile. We already have a few ratified RISC-V profiles, but so far they are treated as "pretty names" (or non-canonical abbreviations) for an ISA base plus a set of extensions. Further, most of the extensions of a profile are irrelevant for toolchain components (e.g. supervisor mode extensions) and thus it is not clear how to decompose profiles into an tools-compatible arch string, that allows to reliably reconstruct the profile that a piece of software was written for (i.e., the profile that the execution environment is expected to implement). This commit attempts to resolve this situation by defining a new attribute that allows to store the target profile. The string representation is used to ensure a future-proof encoding. This should not be a performance issue, since profile names are typically very short (less than 10 characters). This commit also provides a few hints for linkers and run-times, but does not mandate anything. Signed-off-by: Christoph Müllner <[email protected]>
One typical RISC-V profile question is came to my mind again: what if we have enable vendor extension? e.g. RVA22U64+xfoo, does it conformance with RVA22U64? If it conformance it should means we can run the binary on any platform which is RVA22U64 conformance, but it's not true since not every core has implement xfoo extension, however it could claim conformance IF those functions which use xfoo extension are always guarded by some predictor/check. If it not conformance...okay, that's easier, that means we should not set this attribute. |
The profile spec uses the term So, yes, if a vendor extension (or any other non-profile extension) is part of the arch string, then the ELF cannot be compatible with a profile. However, as mentioned in the PR, there is still the option to add optional support for these extensions, but To make this possible, we probably need something like |
Thanks for your clarification, one question come to my mind is: do we really need I guess this should also discuss in riscv-non-isa/riscv-toolchain-conventions#36, but anyway, let us continue the discussion here first :P |
Using profiles as pretty names (that expand to a list of extensions) in an march string is reasonable syntactic sugar. However, A related question is how to set the profiles in the tools (e.g. A proposal for implementing the
|
Just want to add a note here that the existing C flag ( Is the purpose of this ELF attribute to be:
I think that ought to be clearer to avoid the mistakes made with the existing flag. Edit: Another question - is the attribute informational? Or is it intended that the linker or loader actually does something with the flag, such as [examples] refusing to link different objects in some circumstances, or refusing to load a binary that doesn't conform to a profile? |
I'd like to say that while marking ABI changing options that way is perhaps useful, for ABI compatible ones it can cause much more harm than the benefits. Just a recent example is when I've added hw accelerated SHA1 support |
I don't think that Also, Further, there are two observations which are valid for all extensions (not only C):
The first observation should be obvious (if there is no opportunity to use an available instruction, then it can't be used). The second observation comes from override mechanisms (like
No. Toolchain configuration is only used to set defaults (i.e., to define a baseline).
Yes, the build process documents the intended environment for which this ELF was built.
This goes beyond this proposal:
That's impossible to say:
I'm not sure what the mistake was in your opinion. |
That's definitely the theory, but the practical reality is that it doesn't work that way. See the link. (This may be a reason to fix the implementation, but my point is really that we need to very precisely define the meaning of the new attribute, how it is used, and what it is for to avoid implementations being broken as they are for RVC right now). Re ifuncs etc. What does the profile attribute mean for code which is using ifuncs? Say that code does use hwprobe and has an optional vector path. The compiler would probably automatically mark the ELF library as "RVA23" (where I assume V will be mandatory). But it would perfectly well run on RVA22 (where V is optional). Should the runtime reject the program marked "RVA23" even though it could run? Should the toolchain be smart and detect ifuncs (and how?). Would we require library writers to hand-annotate such libraries?
That seems vague. I guess in practice we'd use it for informational purposes. (Edit: To be clear I'm not arguing against this proposal, an informational attribute could be useful, but saying that it could be more useful if we had a clearer idea what it was for, how it would be set, how it would propagate through linking, and who if anyone consumes it) |
@jakubjelinek thanks for sharing the story from Solaris, RISC-V did have record the arch string within ELF object files, and I think so far we have enough stuffs to prevent that become useless, we have e.g. A source code compile with rv64gc, and there is a function come with target attribute to enable vector, then it still mark as rv64gc rather than rv64gcv. // foo won't change the file scope attribute.
int foo () __attribute__(target("arch=+v")){
...
/*
The code gen for foo will be some thing like this:
.option push
.option arch, +v
...
.option pop
*/
}
int main() {
if (vector_is_avaliable)
foo()
} |
I do agree that per-file scoped attributes can have unclear or confusing semantics in the presence of functions compiled for different attributes. I'm not sure I quite follow your concern about the RVC flag in general though. Per the documentation:
That seems consistent with the behaviour you note. As with all the attributes, I can see that you may additionally want to know if any 16-bit aligned instructions were used (or similarly for other ISA extensions - if any instructions from that extension were actually used). But that's outside of scope of what we have today. |
Yes, I was going to feedback that extensions defined as optional in the profile is the big question mark here. If an extension is unconditionally enabled that's defined as optional in the profile, meaning the binary might not run on all RVANN profile systems is there any value in trying to set the profile tag at all? Perhaps it should just be reserved for when targeting the baseline profile. In the common case (i.e. outside of the OS kernel) the S* extensions aren't going to make a difference, so perhaps there's an argument for ignoring those when determinig if the profile tag can be set. |
This PR certainly does not attempt to limit optional extensions (or anything that can be probed at run-time). This PR targets the extensions that are expected to be available. Raising the amount of required extension is not uncommon. E.g. |
The toolchain marks the ELF as RVA23U64 if the toolchain is told to do that. And also no, the toolchain does not care about run-time detection.
Well, being vague is on purpose. I certainly don't want to dictate what the execution environment has to do, because different use-cases might have different requirements. As user I would be happy if a general purpose distro would at least warn me that I am about to execute an i686 binary on an i386 system. In an embedded system that falls out from a Buildroot build such a warning mechanism would be pointless. |
I probably wasn't clear enough here. I mean an object which uses hwprobe / ifunc to offer two paths, one requiring V and one using fallback instructions. The compiler would (probably) automatically mark this as RVA23, or whatever profile requires V, but it would still run without V. The alternative seems to involve manual annotation, which may be fine, but we ought to be clear about the need for this curation. |
I should have used "if and only if" to express this more precisely. Regarding the S* extensions: Initially, I wanted to restrict this PR to RVxUnn profiles (e.g. |
I don't think that the compiler should automatically assume a profile. In your example you need to be aware that RVA23 could also imply auto-vectorization and other reasons to emit vector instructions. So the RVA23 binary is not guaranteed to run on RVA22 machines even if the hand-written vector assembly part is gated by a dynamic discovery mechanism. |
More feedback from the toolchain team at Red Hat:
(Note again while I'm not opposed to this, I don't think it'll prove to be actually useful to anyone) |
Thanks for the comment and the additional references. I want to better understand the "not useful to anyone" argument a bit more. If this interpretation is right, then this PR indeed lacks a strong enough use case to justify its inclusion in the repo. |
I think there are two issues. Firstly as you state, for the distros we either compile all our own software together and certify it on particular hardware, and require third party ISVs to also certify. However there's still some value in having documentation about how an object was built (which is what the Secondly if we want the toolchain or runtime to do something clever - eg. reject binaries which won't work with a nice error message - then we would need something more sophisticated, and also have a good story about how tools like compilers and linkers must generate and propagate these attribute(s). Other arches have tried variations of this before and AFAICT none has been very satisfactory. (Note I'm far from an expert on all this, and I'm shuttling concerns between this issue and the real experts. Will try to get them to participate more in this.) |
This PR was discussed in today's psABI call. Here are the key messages that I got from the (good discussion in the) call:
This PR will be closed sooner or later because it lacks a relevant use case. Thanks for all the good comments on this ticket and the call! |
Just to clarify a few things...
Actually that section contains details on the hardware requirements for the code in the object file. Or you can use a specialized compiler plugin to record the information about the compilation process and store this information somewhere in the object file. This is what the annobin plugin does, although since it is focused on security it only records those options which are related to code hardening. It does not record options related to ABI or ISA selection. (It could do this, it just has not been coded to do so yet).
Annocheck makes use of both information sources (the data recorded by the annobin plugin as well as the DW_AT_producer attribute) as part of its job to ensure that executable binaries have been built with all of the required hardening options enabled. |
Nick thanks for commenting here. How much of annobin is specific to x86-64, and how much would work for other arches (RV64 in this case)? Will it require significant porting work? Will it requires any changes in ELF specs? |
None. :-) Well not quite. The annobin plugin does record details about some x86-64 specific command line options (the ISA selected and the -mstackrealign option) but the plugin and the annocheck tool are mostly architecture agnostic. There are versions of the plugin for all architectures supported by RHEL. It is also worth noting that both the plugin and the annocheck tool can work in a cross compilation environment. So you can cross compile RiscV binaries on, say, an x86_64 host and test them with annocheck on that same x86_64 host or on a native RiscV box.
None - it is already supported. :-) Including a RiscV specific version of the annobin plugin. Although currently no riscv specific command line options are recorded. This could be changed quite easily however. There are builds available for Fedora if you are interested: (And to answer the question more fully, it is fairly simple to add support for other architectures. The annobin sources are small and there are only a few places where target specific additions are needed).
No. Annobin works entirely within the ELF standard and does not need any additions or architecture specific values to be defined. |
This PR implements the draft riscv-non-isa/riscv-toolchain-conventions#36. Currently, we replace specified profile in `-march` with standard arch string. We may need to pass it to backend so that we can emit an ELF attr proposed by riscv-non-isa/riscv-elf-psabi-doc#409.
This PR defines an ELF attribute which allows tools to set a RISC-V target profile.
We already have a few ratified RISC-V profiles, but so far, they are treated as "pretty names" (or non-canonical abbreviations) for an ISA base plus a set of extensions.
Further, most of the extensions of a profile are irrelevant for toolchain components (e.g., supervisor mode extensions). Thus, it is not clear how to decompose profiles into a tools-compatible arch string, that allows to reliably reconstruct the profile that a piece of software was written for (i.e., the profile that the execution environment is expected to implement).
Although this could be solved by implementing "dummy" support for all RISC-V extensions in the tools, but does not feel like a good idea.
This commit attempts to resolve this situation by defining a new attribute that allows storing the target profile.
The string representation is used to ensure a future-proof encoding. This should not be a performance issue since profile names are typically very short (less than 10 characters).
Once the target profile is available, the question about the use of this information comes up.
In this PR, a few hints for linker and run-times are provided, without mandating anything.
The PR has been marked as "RFC", as I'd like to hear not only comments about this particular proposal but also general comments about how to improve the RISC-V profile adoption in the SW ecosystem.