Stop projecting into SIMD types in ui/simd/ tests#138036
Stop projecting into SIMD types in ui/simd/ tests#138036scottmcm wants to merge 1 commit intorust-lang:masterfrom
ui/simd/ tests#138036Conversation
Part of MCP838
rustbot
added
S-waiting-on-review
|
Are u planning on making field access on repr(simd) in HIR a hard error as the next step or what? |
|
@compiler-errors Well, I need to get a stdarch PR in before I can truly ban anything. Locally I've got it banned in the MIR validator (as the fastest way to get an ugly error about it) but a HIR one would certainly be better. EDIT: stdarch PR rust-lang/stdarch#1740 |
compiler-errors
left a comment
compiler-errors
left a comment
There was a problem hiding this comment.
There's nothing wrong w these changes necessarily, but I do feel several tests took a slightly different migration strategy, and they could be made a bit more consistent for better readability.
| let a = $a; | ||
| let b = $b; | ||
| assert_eq!(a.to_array(), b.to_array()); |
There was a problem hiding this comment.
is this necessary? can we not just use $a.to_array()? Also maybe rename this to assert_eq_simd! like the tests below?
| fn to_array(self) -> [i32; N] { unsafe { std::intrinsics::transmute_unchecked(self) } } | ||
| } | ||
|
|
||
| macro_rules! all_eq { |
| for (u, (uu32, ui64)) in u | ||
| .to_array() | ||
| .iter() | ||
| .zip(uu32.to_array().iter().zip(ui64.to_array().iter())) |
There was a problem hiding this comment.
nitpick but u.to_array().into_iter().zip(uu32.to_array()).zip(ui64.to_array()) is a bit simplier 🤔
| let [b, a] = [$b, $a]; | ||
| let [b, a] = [b.to_array(), a.to_array()]; |
There was a problem hiding this comment.
this is pretty awkward lol, is this really necessary for the test?
There was a problem hiding this comment.
let a = $a.to_array();
let b = $b.to_array();
Or else just replace this with the same assert_eq_simd macro as everything else.
There was a problem hiding this comment.
Yeah, this one was weird. As the comment says, it was already doing weird things with type inference. I'll see if I can simplify it...
| let r_strided = simd_gather(default, pointers, mask); | ||
|
|
||
| assert_eq!(r_strided, s_strided); | ||
| assert_eq!(r_strided.to_array(), s_strided.to_array()); |
There was a problem hiding this comment.
macro-wise, should we replace this with assert_eq_simd! rather than calling to_array everywhere?
|
|
||
| macro_rules! all_eq { | ||
| ($a: expr, $b: expr) => {{ | ||
| let a = $a; |
| assert!(const_ptrs.0 == [ptr as *const u8, core::ptr::null()]); | ||
| assert!(exposed_addr.0 == [ptr as usize, 0]); | ||
| assert!(with_exposed_provenance.0 == ptrs.0); | ||
| assert!(const_ptrs.to_array() == [ptr as *const u8, core::ptr::null()]); |
There was a problem hiding this comment.
make this more unified w the rest of the tests
Oh, yeah, MIR validation is probably fine too. It can be an ICE; it doesn't need to be a good diagnostic. |
scottmcm
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #140079) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Closing in favour of #143833 |
rustbot
removed
the
S-waiting-on-author
5 participants
Part of MCP838
This intentionally leaves projections in two places, as a double-check that what stdarch is still using (for now) continues to work
But the things that aren't testing that change to not using
.0on therepr(simd)type.