Conversation
|
Would be good to have tests for these type of stuffs, later on :) |
| } else { | ||
| self.fatal("TODO: Shader copysign not supported yet https://github.com/EmbarkStudios/rust-gpu/issues/148") | ||
| let arg0 = args[0].immediate(); | ||
| let arg1 = args[1].immediate(); |
There was a problem hiding this comment.
I think it would be more clear if you use sign, sign_val, sign_src or something like that instead of arg1.
| ), | ||
| _ => panic!("copysign not supported for width {}", width), | ||
| }; | ||
| let arg0_int = self.bitcast(arg0, int_ty); |
There was a problem hiding this comment.
| let arg0_int = self.bitcast(arg0, int_ty); | |
| let arg0_bits = self.bitcast(arg0, int_ty); |
| }; | ||
| let arg0_int = self.bitcast(arg0, int_ty); | ||
| let arg1_int = self.bitcast(arg1, int_ty); | ||
| let arg0_and = self.and(arg0_int, mask_value); |
There was a problem hiding this comment.
| let arg0_and = self.and(arg0_int, mask_value); | |
| let arg0_masked = self.and(arg0_int, mask_value); |
| let arg1 = args[1].immediate(); | ||
| let width = match self.lookup_type(arg0.ty) { | ||
| SpirvType::Float(width) => width, | ||
| other => panic!( |
There was a problem hiding this comment.
Is panicking appropriate here? Shouldn't this produce a compile error?
There was a problem hiding this comment.
Typeck should already ensure that only floats get passed here. Using unreachable! or bug! makes sense here.
There was a problem hiding this comment.
Ah yeah, I guess I should have used the bug! macro - it's a bug/hard-panic, though, because it's breaking the invariants of what is known today (which is that the arguments to the copysignf32 and copysignf64 functions are floats). Similar with sizes that aren't 32 or 64 bits.
Edit: I guess some relevant context is that rustc_codegen_llvm hits some far worse crashes if intrinsics don't line up, e.g. generating straight-up malformed IL.
There was a problem hiding this comment.
Yeah! I did smoke-test it real quick, but proper tests would be nice. Also oh my goodness, I spent 5 minutes bending my brain trying to think of what |
Fixes #148