-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Reduce dependence on the taget pointer width #27845
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
f276a9e
to
30ec363
Compare
tws => panic!("Unsupported target word size for memcpy: {}", tws), | ||
}; | ||
let ptr_width = &ccx.sess().target.target.target_pointer_width[..]; | ||
let key = format!("llvm.memcpy.p0i8.p0i8.i{}", ptr_width); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of this PR we only have 16, 32 and 64 bit intrinsics bound. The error message will be much worse here now for target_ptr_width ∉ {16, 32, 64}.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in a new commit - the compiler will now panic, saying that the intrinsic is unknown to Rust, along with the intrinsic in question.
If you had previously tried to get the ValueRef associated with an intrinsic that hadn't been described in `trans::context::declare_intrinsic()`, the compile would panic with an empty message. Now we print out details about the error in the panic message.
…richton This patch rewrites code in several places which assume that the current target has either 32-bit or 64-bit pointers so that it can support arbitrary-width pointers. It does not completely remove all assumptions of pointer width, but it does reduce them significantly. There is a discussion [here](https://internals.rust-lang.org/t/adding-16-bit-pointer-support/2484/10) about the change.
This patch rewrites code in several places which assume that the current target has either 32-bit or 64-bit pointers so that it can support arbitrary-width pointers.
It does not completely remove all assumptions of pointer width, but it does reduce them significantly. There is a discussion here about the change.