Fix OpenBSD compilation of wgpu_hal::vulkan::drm
#7810
Conversation
ErichDonGubler
force-pushed
the
erichdongubler-push-tkzwmkwnymlu
branch
2 times, most recently
from
85781c1 to
3216957
Compare
| macro_rules! to_u64 { | ||
| ($expr:expr) => {{ | ||
| #[allow(trivial_numeric_casts)] | ||
| let expr = $expr as u64; | ||
| assert!(size_of_val(&expr) <= size_of::<u64>()); | ||
| expr | ||
| }}; | ||
| } |
There was a problem hiding this comment.
I would like to take this opportunity to express my distaste for the lack of portability motivating this workaround, and the workaround itself. 😫
Otherwise, thanks in advance for reviewing~!
ErichDonGubler
changed the title
*_devid and drm_stat.st_rdev to u64 more forcefullywgpu_hal::vulkan::drm
ErichDonGubler
force-pushed
the
erichdongubler-push-tkzwmkwnymlu
branch
2 times, most recently
from
eab6593 to
f03bfc6
Compare
ErichDonGubler
added
type: bug
|
totally not a rust developer so all this is alien to me, but i can confirm that fixes the thunderbird build failure i was seeing on this code (on OpenBSD, ofc). |
….st_rdev` to `u64` forcefully before comparison
ErichDonGubler
force-pushed
the
erichdongubler-push-tkzwmkwnymlu
branch
2 times, most recently
from
3537920 to
606d284
Compare
| // | ||
| // - `armv7-linux-androideabi`: `dev_t` is `c_ulong`, and `*_devid`s are `dev_t`, but | ||
| // `st_rdev` is `c_ulonglong`. So, we can't just do a `==` comparison. | ||
| // - OpenBSD has `dev_t` on both sides, but is unsigned. Therefore, we can't just use |
There was a problem hiding this comment.
unsigned, and on 32-bits according to @semarie (our resident rust expert)
|
Wait, I thought DRM was a Linux kernel API. The code in In other words, shouldn't this be sufficient? modified wgpu-hal/src/vulkan/mod.rs
@@ -28,6 +28,7 @@ mod adapter;
mod command;
mod conv;
mod device;
+#[cfg(target_os = "linux")]
mod drm;
mod instance;
mod sampler;I'm pretty sure Thunderbird isn't even actually using WebGPU. They just want it to compile. |
|
Oh, hmm, it seems like OpenBSD does have its own implementation of the DRM API, so that it can run GPU drivers that depend on it. For example: |
yes we have the drm layers from the linux kernel, and vulkan apparently works on OpenBSD (never looked into it myself, though) |
Connections
Description
Normally,
libc::stat::st_rdev's type andlibc::makedev's return type are the same: adev_t. However, we've needed to work around cases where this isn't true in the past, breaking comparisons between them in thewgpu_hal::vulkan::drmmodule until a fix was put in place. In #7290, for instance, we needed to work around a weird case in Android where this wasn't true, but converting the*_devidside of the comparison tou64first worked.Now, we've discovered another case (OpenBSD) where both sides of the comparison are
dev_t, but signed. We've only dealt with unsigned integers until now, so our workaround to convert the*_devidside of the comparison breaks down becausei32is not convertible tou64usingFrom::from. Sigh! Time to find something else that works.I think the simplest solution is to use an
ascast. 😱 So, do that here: make ato_u64!(…)macro that force-converts things,asserting that we're not casting something bigger than au64. Then, apply it to both sides of our comparison. Voilà!Also, document this weirdness, so I don't have to write it out in the event of yet another weird divergence for the types in this comparison. 😅
Testing
For CI going forward, 🤷🏻♂️ this is not a configuration we normally care to test! I'm not sure what the right solution here is. My suggestion is that we treat this as a one-off, since this is (AFAIK) the first fix motivated by OpenBSD specifically.
Squash or Rebase?
squush plz
Checklist
CHANGELOG.mdentry.