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

add nvptx vendor intrinsics and initial no_std support #191

Merged
merged 3 commits into from
Nov 17, 2017

Conversation

gnzlbg
Copy link
Contributor

@gnzlbg gnzlbg commented Nov 16, 2017

stdsimd does currently not compile as is with #![no_std].

This PR adds the "nostd" crate feature that enforces #![no_std] and ci test to cover this.

The only dependency stdsimd has on std is std::os::raw::c_void, which I've worked around by c&p c_void into the x86 module.

Once we merge ARM run-time feature detection it will also get a dependency on std::fs. The plan is then to split stdsimd into a core component, and a std component.


The nvptx intrinsics are always available independently of the architecture, but one can only call them from an nvptx kernel.

@BurntSushi
Copy link
Member

This looks OK to me. r? @alexcrichton

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 16, 2017

@alexcrichton I am not suggesting that we should stabilize nvptx intrinsics any time soon, but...

Currently compiling CUDA code using Rust requires three unstable features: abi_ptx, untagged_unions (isn't this stable already?), and platform_intrinsics.

So... if we were to ever stabilize this... that basically cuts it down to abi_ptx.

In the mean time this allows those interested to implement and use these intrinsics on unstable rust without having to use platform_intrinsics or hack into the compiler.

@japaric I have tested this by switching the dependency of your nvptx project kernel crate to use stdsimd instead of nvptx-builtins, but if we were to ever expose these here for "serious" use we would need to figure out a way to use cross to at least tests that kernels compile properly.

@japaric
Copy link
Member

japaric commented Nov 16, 2017

we would need to figure out a way to use cross to at least tests that kernels compile properly.

we don't really need cross to use the nvptx targets though. Emitting NVPTX doesn't require any system library or tool and there's no binary distribution of core for these targets either so Xargo is enough -- provided that you have the target specification file around since these targets are not built-in.

(semi on-topic: I'd like to see cargo build to Just Work and directly generate NVPTX as the output artifact but there's no such thing as a standalone NVPTX linker (there's no NVPTX object forrmat either) -- you can use clang as a linker to link bitcode but that's too big of a dependency. What we could do is something like what's being done for the new wasm target: set obj-is-bitcode to true, force LTO and have LLVM do the linking under the hood.)

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 16, 2017

we don't really need cross to use the nvptx targets though.

Yes, the problem is not compiling, but running the tests.

So what I was thinking is using cross just for running the tests. Instead of using qemu-user we would use, e.g., ocelot, to run the PTX kernels on x86. I haven't used ocelot myself though, so I don't know how good this could work.


(semi on-topic: I'd like to see cargo build to Just Work and directly generate NVPTX as the output artifact but there's no such thing as a standalone NVPTX linker (there's no NVPTX object forrmat either)

Even less on-topic: I'd like to just be able to stamp a #[kernel] attribute to a Rust function or closure, compile my code with cargo build --kernel-targets="x86-avx2,nvptx-sm30", and be able to call the kernel transparently in the CPU and also launch it as a cuda kernel.

But until then a #[kernel_nvptx] attribute just for cuda kernels would already be sweet enough.

@mattico
Copy link

mattico commented Nov 16, 2017

Nit: Cargo features should be additive (couldn't find a better link but Steve has mentioned this), so it'd be better to have a std feature that's enabled by default.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 16, 2017

@mattico thanks, looks better now :)

@gnzlbg gnzlbg merged commit 231405f into rust-lang:master Nov 17, 2017
@@ -32,3 +32,4 @@ cupid = "0.3"

[features]
strict = []
std = []
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be default = ["std"]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this, and I think we should settle it before the next release. I've opened #193.

@@ -128,6 +128,10 @@
cast_possible_truncation, cast_precision_loss,
shadow_reuse, cyclomatic_complexity, similar_names,
doc_markdown, many_single_char_names))]
#![cfg_attr(not(feature = "std"), no_std)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically this is an unconditional #![no_std] if a crate is no_std compatible, but I'll send a PR for this.

@alexcrichton
Copy link
Member

Is there a URL at which we can point the documentation to as well? Something like Intel's source of truth for these intrinsic definitions?

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 19, 2017

@alexcrichton probably the CUDA API docs are the closest thing to that. The problem is that CUDA considers these to be part of the CUDA language, we only see them as intrinsics here because CUDA Rust is not a thing.

So I am going to link those there and also link the docs of the LLVM's NVPTX backend but I will mention that these intrinsics are experimental and if you don't know what they do you probably shouldn't be using them.

I still think this is pretty exciting. If we get @japaric's https://github.com/japaric/cuda and nvptx libraries to work on top of this, we might one day offer CUDA on stable Rust "as a library" by just stabilizing these.

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

Successfully merging this pull request may close these issues.

5 participants