Skip to content
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

Enhancement: Add support for --target-cpu=native #21

Open
itamarst opened this issue Jul 6, 2023 · 3 comments
Open

Enhancement: Add support for --target-cpu=native #21

itamarst opened this issue Jul 6, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@itamarst
Copy link

itamarst commented Jul 6, 2023

For local development, or just fixed hardware, it would be nice to take advantage of the fact that one is not distributing the binaries but compiling them locally. Being able to add --target-cpu=native to RUSTFLAGS would make some code even faster.

This is particularly helpful in numeric code where you can get the compiler to use SIMD automatically, making rustimport a stronger alternative to Numba, which already takes advantage of current CPU features by default.

It's unclear whether this should be on by default. Certainly you want it off anytime you are building on one machine and running the resulting code an arbitrary different machine, because you might end up generating code that won't run on other CPUs with different instruction sets (e.g. not all x86-64 CPUs have AVX-512 or AVX2).

@mityax
Copy link
Owner

mityax commented Jul 13, 2023

That's a neat idea, thank you!

It's unclear whether this should be on by default. Certainly you want it off anytime you are building on one machine and running the resulting code an arbitrary different machine, because you might end up generating code that won't run on other CPUs with different instruction sets (e.g. not all x86-64 CPUs have AVX-512 or AVX2).

I think a reasonable approach would be to enable this by default when builds are triggered by the import hook, but not when the project is built from the command line, since this could indicate a build in e.g. a docker container. However, adding a command line flag for this could still enable users to easily opt-in if they wish to.

I'm not entirely sure yet whether I think this should be a part of rustimport, since it can always be done by setting the env var manually. But I do see that it nicely fits into the semantics of an import hook.

I'll think more about this in about a month, since I don't have enough time atm – in the mean time feel free to create a PR, if you'd like to.

@mityax mityax added the enhancement New feature or request label Jul 13, 2023
@itamarst
Copy link
Author

I tried setting RUSTFLAGS and it didn't seem to work? But that might just be user error on my part, I can try again.

@mityax
Copy link
Owner

mityax commented Jul 13, 2023

Yeah, that'd be nice. Since rustimport is using subprocess.Popen to invoke cargo, which makes it inherit the env your script is started with by default, it should work.
Also, it has already been working with ARCHFLAGS (see #23)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants