Remove Unsafe in macro.rs #43
Conversation
this should be a way to remove the unsafe code in the Spirv Deref impl. Not sure how it affects performance yet. I will take a look at the bytecode and see if maybe the compiler is smart enough to optimize that away.
Let the deref return a `Vec<u32>` instead of a slice reference. This should not hurt performance too much as this will be done a finite amount of times att startup.
|
Even the original did not account for endienness mismatch. So I think this
should not break on any system that it previously worked on.
bjorn3 <notifications@github.com> schrieb am Do., 7. Nov. 2019, 23:53:
… ***@***.**** commented on this pull request.
------------------------------
In src/macros.rs
<#43 (comment)>:
> @@ -27,13 +27,18 @@ macro_rules! include_spv {
/// The definition of this type is unstable.
pub type Spirv = Align4<[u8]>;
-impl std::ops::Deref for Spirv {
- type Target = [u32];
- fn deref(&self) -> &[u32] {
- #[allow(clippy::cast_ptr_alignment)]
- unsafe {
- std::slice::from_raw_parts(self.0.as_ptr() as *const u32, self.0.len() / 4)
+impl Spirv {
+ pub fn deref(&self) -> Vec<u32> {
+ let mut out = Vec::with_capacity(self.0.len() / 4);
+ for i in 0..self.0.len() / 4 {
+ let mut tmp: u32 = 0;
+ tmp += self.0[4 * i] as u32;
+ tmp += (self.0[4 * i + 1] as u32) << 8;
+ tmp += (self.0[4 * i + 2] as u32) << 16;
+ tmp += (self.0[4 * i + 3] as u32) << 24;
This will have different behaviour compared to the original on big endian
systems.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#43?email_source=notifications&email_token=AG26S7YF4RSLGDDB36V3L5LQSSMABA5CNFSM4JKO7MVKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCKZI3MA#pullrequestreview-313691568>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AG26S73GAEEJERLRRUVG3OTQSSMABANCNFSM4JKO7MVA>
.
|
|
In addition, we can also add |
|
Ah ok I did not see that. Why did you choose to go the unsafe route then? Performance? |
|
No, it was contributed by Ralith in their PR for ash: ash-rs/ash#245 |
|
Ok i now have used Do we want/need to throw that output Vec into |
|
The |
|
Closes #40 |
This is my take on removing the unsafe block in macro.rs.
This will definitely be slower than the unsafe version but I don't think that it matters too much as it will not be called very often.
I could not get it work with the Deref impl so I changed that to a normal function.