rustimport icon indicating copy to clipboard operation
rustimport copied to clipboard

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

Open itamarst opened this issue 2 years ago • 3 comments

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).

itamarst avatar Jul 06 '23 17:07 itamarst

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 avatar Jul 13 '23 13:07 mityax

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.

itamarst avatar Jul 13 '23 15:07 itamarst

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)

mityax avatar Jul 13 '23 15:07 mityax