-
Notifications
You must be signed in to change notification settings - Fork 709
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
"mips-" support still missing? ("mipsel-" compiles fine) #1866
Comments
Seems the infra is already there, and one only needs to teach the build system to recognize big-endian MIPS. |
PR #1859 was submitted to add the big-endian variant. I asked for a couple of changes. If somebody updates that PR or submits a new one with the requested changes made, I will be happy to merge it. |
Thanks @xen0n and @briansmith for your quick responses. I'm happy to work on the requested changes for #1859. If I'm not mistaken, this is what you are asking for: brocaar@9467d8c. However, it looks like #1859 might still have some issues. For my project I'm using
Because of Full log
Any idea if there is something else that needs to be changed? |
I'm updating PR 1859 with requested changes. Sorry about the delay - other priorities took precedence. |
@brocaar Are you building optimized for size? And/or using PGO? One thing I can say is that if you are using AES at all then these functions are definitely not "unlikely" and AFAICT really do benefit a lot from inlining, so perhaps it is worth it for us to try to convince the compiler to do the inlining against its own judgement. |
@briansmith, you are right, this was triggered by |
I had this command |
Does anybody here have a suggestion for how we can annotate the code in aes_nohw.c to tell the compiler that, even if the compiler optimization setting is to optimize for size by default, that it should do this inlining and not worry if it would grow the code? |
@briansmith I'm not sure if this is related to my development environment or applies to all, but I noticed that when using |
@brocaar the difference you might have noticed is that referencing crates.io uses prebuilt version of ring. If you patch it (to point to a git reference or local drive) you will cause code to be built - and any compiler warnings generated. |
@briansmith |
We only enable |
New PR #1894 raised for MIPS BE support. |
I merged a slightly tweaked versoin of @DavidHorton5339's PR as part of #1910. I also added all the supported MIPS targets to ring's CI, tested only on the Rust 1.71.0 toolchain. I expect this will be released as part of ring 0.17.8 next week. |
@briansmith any word on that 0.17.8 release with these changes on crates.io? |
To enable compilation for MIPS64, you only need to add the following code under the existing code in #elif defined(__MIPSEB__) && !defined(__LP64__)
#define OPENSSL_32_BIT Add this: #elif defined(__MIPSEB__) && defined(__LP64__)
#define OPENSSL_64_BIT This will configure the system to use MIPS64. |
Testing with ring v0.17.7, compiling for
mipsel-
works fine (e.g.mipsel-unknown-linux-musl
), however compiling formips-
(e.g.mips-unknown-linux-musl
) returns with an error:Full error
@briansmith, looking at https://github.com/briansmith/ring/blob/main/include/ring-core/target.h#L40, I only see
__MIPSEL__
definitions. Looking at #562 (comment), it looks like issue #562 was intended to also implement themips-
targets, but maybe this has changed or this was overlooked?The text was updated successfully, but these errors were encountered: